Opened 4 years ago

Closed 3 years ago

#1650 closed defect (fixed)

Additional connections during tunnel opening are rejected

Reported by: zzz Owned by: zzz
Priority: major Milestone: 0.9.23
Component: api/i2cp Version: 0.9.21
Keywords: Cc: dg
Parent Tickets:

Description

If an application attempted multiple connections and the tunnel wasn't open yet, it used to be that all connections would block until the tunnel opened.

As of 0.9.21, only the first one blocks and the others are rejected. This change was a side-effect of changes for multisession.

I2PSession.isClosed() now returns false when tunnel open is in progress. So streaming I2PSocketManagerFull.verifySession() doesn't call session.connect, where the 2nd connection would block.

I changed the behavior to fix some problems with multisession but don't recall the details, only realized later that this got broken. isClosed() may not be sufficient, may need a new method/API for various states. May not be a simple fix as lots of places depend on isClosed() to work one way or the other. Combined with other changes in .21 to throw I2PSessionException when sending before the session is ready, the current behavior is the client socket gets closed immediately.

Can reproduce with a delay-open or close-on-idle IRC client tunnel and an IRC client with two default servers (e.g. IRC2P and kytv). Could also affect web browsing.

Beware of breaking stuff when attempting to fix this.

Subtickets

Change History (4)

comment:1 Changed 4 years ago by dg

  • Cc dg added

Does affect web browsing.

<+dg> zzz: it looks like with a close-on-idle HTTP proxy, the first connection is held open and is given the data once the tunnels are built, while any further connections before they're built (say you open two tabs at once) will be rejected immediately?
<+dg> is that worth putting on trac?
<+dg> using privoxy.
<&zzz> dg ticket #1650
<+iRelay> http://trac.i2p2.i2p/ticket/1650 - new defect  - Additional connections during tunnel opening are rejected
<+dg> huh. knew i'd read it. thanks

comment:2 Changed 3 years ago by zzz

  • Milestone changed from undecided to 0.9.23
  • Owner set to zzz
  • Priority changed from minor to major
  • Status changed from new to accepted

comment:3 Changed 3 years ago by zzz

Rather than add a new isReallyNotClosed() or similar, I'm going to have sendMessage() block if opening is in progress. This will be the safest way forward. Testing now.

comment:4 Changed 3 years ago by zzz

  • Resolution set to fixed
  • Status changed from accepted to closed

in 8ac7c6dcfbf0c2fe4168539cc75ed601a9a6349c 0.9.22-23

Note: See TracTickets for help on using tickets.