Opened 9 years ago

Closed 9 years ago

#5146 closed Bug (fixed)

seeding complete callback called twice

Reported by: livings124 Owned by: jordan
Priority: Normal Milestone: 2.80
Component: libtransmission Version: 2.73
Severity: Normal Keywords:
Cc:

Description (last modified by livings124)

from #5106

There is a bug in libtransmission where the ratio or idle limit callbacks are called twice quickly (less than a second apart).

Attachments (1)

5146_exp_fix.patch (906 bytes) - added by x190 9 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 9 years ago by livings124

  • Description modified (diff)

comment:2 Changed 9 years ago by cfpp2p

I might not think there is a problem here really, but...

tr_torrentCheckSeedLimit() is the call from tr_torrentRecheckCompleteness()

tr_torrentRecheckCompleteness() called from bandwidthPulse() and also line 1122 torrent.c

tr_torrent_activity
tr_torrentGetActivity( tr_torrent * tor )
{
    /* FIXME: is this call still needed? */
    tr_torrentRecheckCompleteness( tor );

    return torrentGetActivity( tor );
}

since r13622 didn't fix #5106 https://trac.transmissionbt.com/ticket/5106#comment:33 anyway this ticket seems out of place to me.

comment:3 Changed 9 years ago by livings124

This is still a bug regardless if #5106 is fixed. When a downloading torrents hits a ratio, the ratio callback should be called once, not twice.

comment:4 Changed 9 years ago by cfpp2p

relevant information is posted here: #5106 and https://trac.transmissionbt.com/ticket/5106#comment:37

comment:5 Changed 9 years ago by x190

The following occurs near-simultaneously on torrent completion when the ratio-limit is hit at the same time.
https://forum.transmissionbt.com/download/file.php

The incorrect ordering and the second "Seeding Complete" are causing the crashes reported in #5106.

Can Line 2086 fireCompletenessChange(); in tr_torrentRecheckCompleteness() [torrent.c] be moved up to Line 2061 s.t. it will be called first?

Last edited 9 years ago by x190 (previous) (diff)

comment:6 Changed 9 years ago by cfpp2p

First of all transmission is not real time interrupt driven processing so there isn't any simultaneous anything. One function's processing can take place immediately after another's but it is all linear processing, nothing simultaneously.

please follow this at: https://trac.transmissionbt.com/ticket/5106#comment:41

comment:7 Changed 9 years ago by cfpp2p

The incorrect ordering and the second "Seeding Complete" are causing the crashes reported in #5106.

The second "Seeding Complete" is caused by the: tr_torrentSetRatioLimitHitCallback(fHandle, ratioLimitHitCallback, self); line in the Mac code https://trac.transmissionbt.com/ticket/5106#comment:41

Changed 9 years ago by x190

comment:8 Changed 9 years ago by x190

Fixes order of notifications and eliminates duplicates. Please test. Patch to r13622.

Last edited 9 years ago by x190 (previous) (diff)

comment:9 Changed 9 years ago by livings124

If anyone can get breakpoints working, can you supply backtraces from where the three callbacks are called?

comment:11 Changed 9 years ago by jordan

  • Milestone changed from None Set to 2.80
  • Status changed from new to assigned

livings124, thanks for the backtraces. The behavior they're showing looks like it ought to be fixed by x190's patch.

x190, nice work :)

comment:12 Changed 9 years ago by jordan

r13627 adds x190's patch.

livings124, since the Mac client is the one using this code, could you confirm the fix? Please close the ticket if everything looks good to you.

comment:13 Changed 9 years ago by livings124

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

The callback is only fired once now. Thanks!

comment:14 Changed 9 years ago by jordan

  • Milestone changed from 2.80 to 2.74

comment:15 Changed 9 years ago by jordan

  • Resolution fixed deleted
  • Status changed from closed to reopened

Actually, moving the Completeness Callback to this point in the file makes it less useful. You can't reliably look at filenames right after this callback because the libtransmission thread may still be renaming them and/or moving them from the incomplete to complete directory.

Looking at livings124's callbacks in comment:10, I think it would be safe to move the callback to where it was before r13627. The callback in http://pastebin.com/50JLTySV shouldn't be happening in trunk anymore because, as of r13651, tr_torrentGetActivity() doesn't call tr_torrentRecheckCompleteness() anymore.

comment:16 Changed 9 years ago by jordan

  • Milestone changed from 2.74 to 2.80

comment:17 Changed 9 years ago by jordan

r13880: revert r13627.

livings124, please close this ticket if the seeding callback still isn't being called twice. Thanks.

comment:18 Changed 9 years ago by livings124

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

Looks to be working to me.

Note: See TracTickets for help on using tickets.