Opened 7 years ago

Closed 7 years ago

#675 closed defect (fixed)

Various UDPPacket "leaks"

Reported by: zab Owned by: zzz
Priority: minor Milestone: 0.9.2
Component: router/transport Version: 0.9.1
Keywords: SSU OOM leak Cc:
Parent Tickets:

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.

  1. 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))
    
    
  1. 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;
             }
    
    
  1. 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 7 years ago by zzz

  • Milestone set to 0.9.2
  • Priority changed from major to minor
  • Resolution set to fixed
  • Status changed from new to closed

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.

Note: See TracTickets for help on using tickets.