Opened 14 months ago
Closed 14 months ago
#2686 closed defect (fixed)
EventPumper.java: High CPU, leaks memory
Reported by: | jogger | Owned by: | zzz |
---|---|---|---|
Priority: | major | Milestone: | 0.9.45 |
Component: | router/transport | Version: | 0.9.44 |
Keywords: | Cc: | ||
Parent Tickets: | Sensitive: | no |
Description
The bug is around runDelayedEvents() looping over _wantsWrite, when closed connections with null _conKey get stuck there forever.
The failsafe code does not catch this. As a quick fix one could loop over _wantsWrite during failsafe to remove closed cons or replace line 821 with
if ((con = iter.next()).isClosed()) { iter.remove(); continue; };
Also a list would be more efficient than a hash set. Frequently modified hash maps grow indefinitely large over time even when nearly empty and should be disposed of reqularly.
Subtickets
Change History (4)
comment:1 Changed 14 months ago by
Milestone: | undecided → 0.9.45 |
---|---|
Status: | new → accepted |
comment:2 Changed 14 months ago by
I have not fully gone through all possible cases. If the (key == null) test is there because of a pending write waiting for the conkey to be set then the patches act differently. Otherwise yours is better of course.
comment:3 Changed 14 months ago by
Sorry for the short answer. I coded the patch because of this found in router/java/src/net/i2p/router/transport/ntcp/NTCPConnection.java.
The connections found in the heap dump were outbound.
* Valid for inbound; valid for outbound shortly after creation */ public synchronized SelectionKey getKey() { return _conKey; }
comment:4 Changed 14 months ago by
Resolution: | → fixed |
---|---|
Status: | accepted → closed |
isClosed() is probably sufficient but to be safe, left the null check in there too.
But I don't see how the key could be null, wantsWrite() shouldn't be called before the connection happens and the key is set.
In e1bf50e69f3df6c9fca5547052ea6d73b692246f 0.9.44-8-rc
How about this? Testing now.