Opened 8 years ago
Closed 8 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: | Zlatin Balevsky | |
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 8 years ago by
comment:2 Changed 8 years ago by
Component: | unspecified → router/transport |
---|---|
Milestone: | 0.9.7 → 0.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 8 years ago by
Cc: | Zlatin Balevsky added |
---|
Here's a stab at this http://pastethis.i2p/show/4166/
comment:4 Changed 8 years ago by
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 8 years ago by
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 8 years ago by
And another stab, this time not holding on to the large EstablishSate? object http://pastethis.i2p/show/4174/
comment:7 Changed 8 years ago by
Small bug in the Reader loop, fix: http://pastethis.i2p/show/4176/
comment:9 follow-up: 10 Changed 8 years ago by
After some suggestions from zzz http://pastethis.i2p/show/4179/
comment:10 follow-up: 11 Changed 8 years ago by
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:12 Changed 8 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Should be merged to trunk with revision 07a52fb185cd31462111fef287cc1ea1b7da09a4
version 0.9.7-4
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