Opened 9 years ago

Closed 9 years ago

#663 closed defect (fixed)

UDPPacket.verifyNotReleased() logic inverted

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


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 9 years ago by Zlatin Balevsky

Summary: UDPPacket.verifyNotRelease() logic invertedUDPPacket.verifyNotReleased() logic inverted

comment:2 Changed 9 years ago by Zlatin Balevsky

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 9 years ago by zzz

Milestone: 0.9.2
Status: newaccepted

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 9 years ago by zzz

Resolution: fixed
Status: acceptedclosed

Fixed in 0.9.1-2 with level changed to ERROR.

Note: See TracTickets for help on using tickets.