Opened 9 years ago
Closed 9 years ago
#675 closed defect (fixed)
Various UDPPacket "leaks"
Reported by: | Zlatin Balevsky | Owned by: | zzz |
---|---|---|---|
Priority: | minor | Milestone: | 0.9.2 |
Component: | router/transport | Version: | 0.9.1 |
Keywords: | SSU OOM leak | Cc: | |
Parent Tickets: | Sensitive: | no |
Description
In several places in the code UDPPacket's do not get release()-ed. This makes limiting the memory usage and preventing OOMs as suggested in ticket #674 harder.
- Hole punch packets in UDPReceiver.Runner.run():
# # old_revision [458691a358f26fca9ecca1810381ce7d843e995b] # # patch "router/java/src/net/i2p/router/transport/udp/UDPReceiver.java" # from [26cd34990b12acd2de0b7237b74327b618ade484] # to [7d76285c7df5306b8fe17b02ab0c961fb5199213] # ============================================================ --- router/java/src/net/i2p/router/transport/udp/UDPReceiver.java 26cd34990b12acd2de0b7237b74327b618ade484 +++ router/java/src/net/i2p/router/transport/udp/UDPReceiver.java 7d76285c7df5306b8fe17b02ab0c961fb5199213 @@ -259,6 +259,7 @@ class UDPReceiver { int queued = receive(packet); _context.statManager().addRateData("udp.receivePacketSize", size, queued); } else { + packet.release(); _context.statManager().addRateData("udp.receiveHolePunch", 1, 0); // nat hole punch packets are 0 bytes if (_log.shouldLog(Log.INFO))
- Old packets and POISON packets in UDPSender.getNextPacket():
# old_revision [458691a358f26fca9ecca1810381ce7d843e995b] # # patch "router/java/src/net/i2p/router/transport/udp/UDPSender.java" # from [331b343409a02317a050f41d60b54425b7413e34] # to [216bf9cb757679ef445478953dce2f694555e0b7] # ============================================================ --- router/java/src/net/i2p/router/transport/udp/UDPSender.java 331b343409a02317a050f41d60b54425b7413e34 +++ router/java/src/net/i2p/router/transport/udp/UDPSender.java 216bf9cb757679ef445478953dce2f694555e0b7 @@ -250,14 +250,22 @@ class UDPSender { /** @return next packet in queue. Will discard any packet older than MAX_HEAD_LIFETIME */ private UDPPacket getNextPacket() { UDPPacket packet = null; - while ( (_keepRunning) && (packet == null || packet.getLifetime() > MAX_HEAD_LIFETIME) ) { + while ( (_keepRunning) && (packet == null)) ) { if (packet != null) _context.statManager().addRateData("udp.sendQueueTrimmed", 1, 0); try { packet = _outboundQueue.take(); } catch (InterruptedException ie) {} - if (packet != null && packet.getMessageType() == TYPE_POISON) + if (packet == null) + continue; // really not sure if this is ever possible + if (packet.getMessageType() == TYPE_POISON) { + packet.release(); return null; + } + if (packet.getLifetime() > MAX_HEAD_LIFETIME) { + packet.release(); + packet = null; + } } return packet; }
- Various shutdown() states. Does the transport ever get restarted within the same JVM after it gets shutdown? If so those need fixing too, but since they're more convoluted I'm not including suggestions here.
Subtickets
Change History (1)
comment:1 Changed 9 years ago by
Milestone: | → 0.9.2 |
---|---|
Priority: | major → minor |
Resolution: | → fixed |
Status: | new → closed |
Note: See
TracTickets for help on using
tickets.
1) was fixed in (about) 0.9.1-6.
2a) (old packets) was fixed at the same time
2b) (poison packets) only happens at shutdown
3) shutdown, should not be fixed, as we want them to get GC'ed. We clear the cache at shutdown to minimize memory usage. The only case where a router gets restarted in the same JVM is Android. Except for "multi-router" testing in the same JVM, that nobody ever does.
Closing ticket, as 1) and 2a) are fixed, and 2b) and 3) won't be fixed.