Opened 6 years ago

Closed 6 years ago

#972 closed defect (fixed)

Error in the ntcp reader: java.lang.RuntimeException

Reported by: DISABLED Owned by: zzz
Priority: minor Milestone: 0.9.9
Component: router/transport Version: 0.9.6
Keywords: Cc: zab@…
Parent Tickets: Sensitive: no

Description

10/07/13 05:30:26 CRIT [P reader ¼] p.router.transport.ntcp.Reader: Error in the ntcp reader

java.lang.RuntimeException?: connection was not established, yet the establish state is null for NTCP conn from unknown not established created 187ms ago
at net.i2p.router.transport.ntcp.Reader.processRead(Reader.java:147)
at net.i2p.router.transport.ntcp.Reader.access$400(Reader.java:21)
at net.i2p.router.transport.ntcp.Reader$Runner.run(Reader.java:118)
at java.lang.Thread.run(Thread.java:722)
at net.i2p.util.I2PThread.run(I2PThread.java:85)

Subtickets

Change History (12)

comment:1 Changed 6 years ago by DISABLED

I2P version: 0.9.6-0
Java version: Oracle Corporation 1.8.0-ea (Java™ SE Runtime Environment 1.8.0-ea-b36e)
Wrapper version: 3.5.17
Server version: 7.6.10.v20130312
Servlet version: Jasper JSP 2.1 Engine
Platform: Linux arm 3.6.11+
Processor: uninitialized (arm)
Jbigi: Locally optimized native BigInteger? library loaded from file
Encoding: UTF-8
Charset: UTF-8

comment:2 Changed 6 years ago by zzz

Component: unspecifiedrouter/transport
Milestone: 0.9.70.9.9
Owner: set to zzz

This is some sort of really ugly race in NTCP. Needs research. Probably shouldn't be a CRIT.

comment:3 Changed 6 years ago by Zlatin Balevsky

Cc: zab@… added

Here's a stab at this http://pastethis.i2p/show/4166/

comment:4 Changed 6 years ago by zzz

I don't think that's sufficient, or perhaps even on the right track. Isn't the real problem in Reader.processRead() which is looping and checking multiple things (isClosed(), isEstablished(), getNextReadBuf()) - without any locking at all?

But back in NTCPConnection, _establishState isn't final and doesn't even get created in the outbound constructor - and so the "shouldn't happen" comment in prepareNextWriteFast() is wrong? Then the establishState gets nulled out after it's connected, which is fine I guess.

Just another example of crappy state management with several different fields and methods to get them.

Not clear why processRead() has to know about the establisher at all. I guess the idea is that if the handshakes are happening, the packets go the establisher, not to the connection itself via recvEncryptedI2NP().

Seems like a lot of it could be cleaned up by moving most of processRead() to NTCPConnection, and let NTCPConnection worry about its own state and routing incoming packets to either the estabisher or recvEncryptedI2NP, or throw/log/drop/etc. Then Reader doesn't have to use multiple non-locked NTCPConnection methods to determine the current state and try to do the right thing.

comment:5 Changed 6 years ago by Zlatin Balevsky

I agree with all the points above. There is no single clean solution. Here is another try that incorporates some of the suggestions: http://pastethis.i2p/show/4172/

comment:6 Changed 6 years ago by Zlatin Balevsky

And another stab, this time not holding on to the large EstablishSate? object http://pastethis.i2p/show/4174/

comment:7 Changed 6 years ago by Zlatin Balevsky

Small bug in the Reader loop, fix: http://pastethis.i2p/show/4176/

comment:8 Changed 6 years ago by Zlatin Balevsky

Ignore previous pastes, use http://pastethis.i2p/show/4178/

comment:9 Changed 6 years ago by Zlatin Balevsky

After some suggestions from zzz http://pastethis.i2p/show/4179/

comment:10 in reply to:  9 ; Changed 6 years ago by Zlatin Balevsky

Replying to zab:

After some suggestions from zzz http://pastethis.i2p/show/4179/

07:14:23:54:14:218 ERROR [NTCP Pumper ] ter.transport.ntcp.EventPumper: Error in the event pumper
java.lang.NullPointerException
        at net.i2p.router.transport.ntcp.EventPumper.runDelayedEvents(EventPumper.java:754)
        at net.i2p.router.transport.ntcp.EventPumper.run(EventPumper.java:170)
        at java.lang.Thread.run(Thread.java:722)
        at net.i2p.util.I2PThread.run(I2PThread.java:85)

comment:11 in reply to:  10 Changed 6 years ago by Zlatin Balevsky

Replying to zab:
Addressed in http://pastethis.i2p/show/4182/

comment:12 Changed 6 years ago by Zlatin Balevsky

Resolution: fixed
Status: newclosed

Should be merged to trunk with revision 07a52fb185cd31462111fef287cc1ea1b7da09a4
version 0.9.7-4

Note: See TracTickets for help on using tickets.