Opened 6 months ago

Closed 6 months ago

#2777 closed defect (fixed)

NTCP2: terminations not sent

Reported by: jogger Owned by: zzz
Priority: minor Milestone: 0.9.48
Component: router/transport Version: 0.9.47
Keywords: Cc:
Parent Tickets: Sensitive: no

Description

Eventpumper cancels the conkey directly after calling sendTerminationAndClose(). This way the termination is only queued up for writing and never sent, because runDelayedEvents() runs into a CKE.

Subtickets

Change History (6)

comment:1 Changed 6 months ago by zzz

Milestone: undecided0.9.48
Status: newaccepted

comment:2 Changed 6 months ago by zzz

something like this maybe? untested:

--- router/java/src/net/i2p/router/transport/ntcp/EventPumper.java	c7d44fae6ee341ea5daadcabefd4f25473af1651
+++ router/java/src/net/i2p/router/transport/ntcp/EventPumper.java	cc0992e20fcf098fb6ea1e944430239828694795
@@ -302,8 +302,8 @@ class EventPumper implements Runnable {
                                 if ( con.getTimeSinceSend(now) > expire &&
                                      con.getTimeSinceReceive(now) > expire) {
                                     // we haven't sent or received anything in a really long time, so lets just close 'er up
+                                    // con will cancel the key
                                     con.sendTerminationAndClose();
-                                    key.cancel();
                                     if (_log.shouldInfo())
                                         _log.info("Failsafe or expire close for " + con);
                                     failsafeCloses++;
============================================================
--- router/java/src/net/i2p/router/transport/ntcp/NTCPConnection.java	e8d0c8e426148f905be973c8bea6b787f9e8cbbc
+++ router/java/src/net/i2p/router/transport/ntcp/NTCPConnection.java	e802c08e918fa69503a643819d6e3d5df548a3bb
@@ -1043,8 +1043,8 @@ public class NTCPConnection implements C
 
     /**
      *  NTCP 1 or 2.
-     *  For NTCP1, sends termination and then closes the connection after a brief delay.
-     *  For NTCP2, simply closes the connection immediately.
+     *  For NTCP1, simply closes the connection immediately.
+     *  For NTCP2, sends termination and then closes the connection after a brief delay.
      *
      *  @since 0.9.36
      */
@@ -1066,6 +1066,8 @@ public class NTCPConnection implements C
      *  @since 0.9.36
      */
     private void sendTermination(int reason, int validFramesRcvd) {
+        // So we don't get called again by the event pumper idle closer
+        _lastSendTime = _context.clock().now();
         // TODO add param to clear queues?
         // no synch needed, sendNTCP2() is synched
         if (_log.shouldInfo())

comment:3 Changed 6 months ago by zzz

Verified OP issue:

10/09 xx:40:18.155 INFO  [NTCP Pumper ] .transport.ntcp.NTCPConnection: Sending termination, reason: 2, vaild frames rcvd: 1 on NTCP2 conn 38388 to xx port xx DGZzTX created 121s ago, last send 121s ago, last recv 120s ago, sent 3, rcvd 1
10/09 xx:40:18.155 WARN  [NTCP Pumper ] ter.transport.ntcp.EventPumper: RDE CKE 2
java.nio.channels.CancelledKeyException
	at sun.nio.ch.SelectionKeyImpl.ensureValid(SelectionKeyImpl.java:73)
	at sun.nio.ch.SelectionKeyImpl.interestOps(SelectionKeyImpl.java:77)
	at net.i2p.router.transport.ntcp.EventPumper.runDelayedEvents(EventPumper.java:829)
	at net.i2p.router.transport.ntcp.EventPumper.run(EventPumper.java:206)
	at java.lang.Thread.run(Thread.java:748)
	at net.i2p.util.I2PThread.run(I2PThread.java:103)

comment:4 Changed 6 months ago by zzz

Briefly tested above patch, seems to work

comment:5 Changed 6 months ago by zzz

Resolution: fixed
Status: acceptedclosed

comment:6 Changed 6 months ago by zzz

Resolution: fixed
Status: acceptedclosed
Note: See TracTickets for help on using tickets.