Opened 7 years ago

Closed 7 years ago

#663 closed defect (fixed)

UDPPacket.verifyNotReleased() logic inverted

Reported by: zab Owned by: zzz
Priority: major Milestone: 0.9.2
Component: router/transport Version: 0.9
Keywords: SSU Cc:
Parent Tickets:


The following logic is inverted:

private void verifyNotReleased() {
        if (CACHE) return; // < but it should be if (!CACHE)

I'm marking it as major priority since it could be masking concurrency issues of unknown severity.


Change History (4)

comment:1 Changed 7 years ago by zab

  • Summary changed from UDPPacket.verifyNotRelease() logic inverted to UDPPacket.verifyNotReleased() logic inverted

comment:2 Changed 7 years ago by zab

After applying this patch on vanilla trunk http://pastethis.i2p/show/1464/ KillYourTV reported the following report http://pastethis.i2p/show/1467/ which confirms that this inverted logic is in fact masking concurrency bugs in the production build (as opposed to an artifact produced by any of my experimental changes):

after 28 minutes:

7/21/12 10:02:52 AM CRIT [DPReceiver.1] router.transport.udp.UDPPacket: my trace
     at net.i2p.router.transport.udp.UDPPacket.logReleaseReport(
     at net.i2p.router.transport.udp.UDPPacket.verifyNotReleased(
     at net.i2p.router.transport.udp.UDPPacket.getLifetime(
     at net.i2p.router.transport.udp.UDPReceiver.doReceive(
     at net.i2p.router.transport.udp.UDPReceiver.receive(
     at net.i2p.router.transport.udp.UDPReceiver.access$700(
     at net.i2p.router.transport.udp.UDPReceiver$
7/21/12 10:02:52 AM CRIT [DPReceiver.1] router.transport.udp.UDPPacket: previous releaser id 65 name UDP Packet handler 2/5
7/21/12 10:02:52 AM CRIT [DPReceiver.1] router.transport.udp.UDPPacket: double release!

comment:3 Changed 7 years ago by zzz

  • Milestone set to 0.9.2
  • Status changed from new to accepted

Good catch, bug from 2005.

Since it's only logging, won't fix it until after the 0.9.1 release so people don't generate more bug reports.

comment:4 Changed 7 years ago by zzz

  • Resolution set to fixed
  • Status changed from accepted to closed

Fixed in 0.9.1-2 with level changed to ERROR.

Note: See TracTickets for help on using tickets.