transmission-daemon 2.92: segfault in scrape_request_delegate() at announcer.c:1402

[sandrotosi]

Hello,
this has been reported to Debian as http://bugs.debian.org/857511:

1. These segfaults have happened 11 times on my system since Feb 26; the /var/log/syslog* have been rotated so that I can not check whether segfault has happened before Feb 26. These segfaults happened randomly.

   I have collected 6 cores of segfaults. Next description will use core.4 as example.

2. transmission-daemon started on 2017-03-11 09:29:27 using:

   $ ulimit -c unlimited
   $ > ~/tmp/transmission.log ; transmission-daemon --logfile ~/tmp/transmission.log --log-debug
   

   Message in syslog related to core.4 (the segfault on 08:11:32 have no core file saved):

   $ zgrep segfault /var/log/syslog.1
   Mar 11 08:11:32 archon kernel: [  408.155176] transmission-da[2572]: segfault at 0 ip 00007f2850fe4a0c sp 00007f284bfaeaf8 error 4 in libc-2.24.so[7f2850e9c000+195000]
   Mar 11 09:34:01 archon kernel: [ 5356.546761] transmission-da[14443]: segfault at 0 ip 00007f6df7849a0c sp 00007f6df2813af8 error 4 in libc-2.24.so[7f6df7701000+195000]
   

   The attachment is the gdb backtrace generated with following command:

   $ gdb --batch -ex "bt" -ex "bt full" -ex "thread apply all bt full" \
     -ex "quit" transmission-daemon core.4 &> core.4.backtrace-log
   

3. Here are some investigation by myself.

   The request 0x7f6decb08fc0 used by scrape_request_delegate():

   (gdb) p *((const tr_scrape_request *) 0x7f6decb08fc0)
   $1 = {url = 0x0, log_name = "[FILENAME_OF_TORRENT_REMOVED---http://354.354.354.354:75535]", '\000' <repeats 56 times>, info_hash = {
       "{INFO_HASH_REMOVED_1",
       "INFO_HASH_REMOVED_2",
       "INFO_HASH_REMOVED_3",
       '\000' <repeats 19 times> <repeats 61 times>}, info_hash_count = 3}
   

   Note that some text is changed due to private reason. Obviously, segfault happens because of request->url is NULL(0x0) and is compared with "http" using memcmp() (announcer.c:1402).

   The request 0x7f6decb08fc0 is the 2nd element of local variable requests(0x7f6decb08a30) of function multiscrape():

   (gdb) p &((const tr_scrape_request *) 0x7f6decb08a30)[1]
   $4 = (const tr_scrape_request *) 0x7f6decb08fc0
   

   The request 0x7f6decb08fc0 is likely build from tr_tier (tr_ptrArray *) 0x7f6df2813b60)->items[1] as the hash, name and tracker are matched:

   (gdb) p (((tr_tier *) ((tr_ptrArray *)0x7f6df2813b60)->items[1])->tor->info->hash)
   $11 = "{INFO_HASH_REMOVED_1"
   (gdb) p (((tr_tier *) ((tr_ptrArray *)0x7f6df2813b60)->items[1])->tor->info->name)
   $13 = 0x7f6dec3fb110 "FILENAME_OF_TORRENT_REMOVED"
   (gdb) p *(((tr_tier *) ((tr_ptrArray *)0x7f6df2813b60)->items[1])->currentTracker)
   $14 = {key = 0x7f6dec4f5ba0 "http://354.354.354.354:75535",
     announce = 0x7f6dec4f5ae0 "http://354.354.354.354:75535/annonuce", scrape = 0x0,
     tracker_id_str = 0x0, seederCount = -1, leecherCount = -1, downloadCount = -1,
     downloaderCount = 0, consecutiveFailures = 0, id = 59}
   

   The torrent of this tr_tier:

   (gdb) p *(((tr_tier *) ((tr_ptrArray *) 0x7f6df2813b60)->items[1])->tor)
   $15 = {session = 0x560e62f8a920, info = {totalSize = 176553400,
       originalName = 0x7f6dec3fb150 "FILENAME_OF_TORRENT_REMOVED",
       name = 0x7f6dec3fb110 "FILENAME_OF_TORRENT_REMOVED",
       torrent = 0x7f6dec46f990 "/home/kralcyor/.config/transmission-daemon/torrents/FILENAME_OF_TORRENT_REMOVED.7b8d920ba22ab7a5.torrent",
       webseeds = 0x0, comment = 0x7f6dec32f450 "",
       creator = 0x7f6dec3fb190 "rin-pr/0.5.1", files = 0x7f6dec2b2700,
       pieces = 0x7f6dec468c90, trackers = 0x7f6dec46e0e0,
       dateCreated = 1485837788, trackerCount = 68, webseedCount = 0,
       fileCount = 1, pieceSize = 262144, pieceCount = 674,
       hash = "{INFO_HASH_REMOVED_1",
       hashString = "HASH_STRING_REMOVED_1",
       isPrivate = false, isFolder = false}, magicNumber = 95549,
     error = TR_STAT_OK, errorString = '\000' <repeats 127 times>,
     errorTracker = '\000' <repeats 127 times>,
     obfuscatedHash = "OBFUSCATED_HASH_REMOVED_1",
     incompleteMetadata = 0x0, peer_id = "-TR2920-k0igydl6wtr0",
     peer_id_creation_time = 1489195834,
     downloadDir = 0x7f6dec45df80 "DOWNLOAD_DIR_REMOVED",
     incompleteDir = 0x0, infoDictLength = 13587, infoDictOffset = 0,
     currentDir = 0x7f6dec45df80 "DOWNLOAD_DIR_REMOVED",
     blockSize = 16384, blockCount = 10776, lastBlockSize = 15800,
     lastPieceSize = 130488, blockCountInPiece = 16, blockCountInLastPiece = 8,
     completion = {tor = 0x7f6dec46fa10, blockBitfield = {bits = 0x0,
         alloc_count = 0, bit_count = 10776, true_count = 10776,
         have_all_hint = true, have_none_hint = false},
       sizeWhenDoneLazy = 176553400, sizeWhenDoneIsDirty = true,
       haveValidLazy = 0, haveValidIsDirty = true, sizeNow = 176553400},
     completeness = TR_SEED, tiers = 0x7f6dec46f490, dhtAnnounceAt = 1489197288,
     dhtAnnounce6At = 1489197384, dhtAnnounceInProgress = false,
     dhtAnnounce6InProgress = false, lpdAnnounceAt = 1489195767,
     downloadedCur = 0, downloadedPrev = 180364824, uploadedCur = 0,
     uploadedPrev = 1582164452, corruptCur = 0, corruptPrev = 0,
     etaDLSpeedCalculatedAt = 0, etaDLSpeed_Bps = 0, etaULSpeedCalculatedAt = 0,
     etaULSpeed_Bps = 0, addedDate = 1485844188, activityDate = 1488957740,
     doneDate = 1485844394, startDate = 1489195767, anyDate = 1489195767,
     secondsDownloading = 226, secondsSeeding = 401015, queuePosition = 70,
     metadata_func = 0x0, metadata_func_user_data = 0x0, completeness_func = 0x0,
     completeness_func_user_data = 0x0, ratio_limit_hit_func = 0x0,
     ratio_limit_hit_func_user_data = 0x0, idle_limit_hit_func = 0x0,
     idle_limit_hit_func_user_data = 0x0, queue_started_user_data = 0x0,
     queue_started_callback = 0x0, isRunning = true, isStopping = false,
     isDeleting = false, startAfterVerify = false, isDirty = true,
     isQueued = false, magnetVerify = false, infoDictOffsetIsCached = false,
     maxConnectedPeers = 60, verifyState = TR_VERIFY_NONE, lastStatTime = 0,
     stats = {id = 0, activity = TR_STATUS_STOPPED, error = TR_STAT_OK,
       errorString = '\000' <repeats 511 times>, recheckProgress = 0,
       percentComplete = 0, metadataPercentComplete = 0, percentDone = 0,
       seedRatioPercentDone = 0, rawUploadSpeed_KBps = 0,
       rawDownloadSpeed_KBps = 0, pieceUploadSpeed_KBps = 0,
       pieceDownloadSpeed_KBps = 0, eta = 0, etaIdle = 0, peersConnected = 0,
       peersFrom = {0, 0, 0, 0, 0, 0, 0}, peersSendingToUs = 0,
       peersGettingFromUs = 0, webseedsSendingToUs = 0, sizeWhenDone = 0,
       leftUntilDone = 0, desiredAvailable = 0, corruptEver = 0,
       uploadedEver = 0, downloadedEver = 0, haveValid = 0, haveUnchecked = 0,
       manualAnnounceTime = 0, ratio = 0, addedDate = 0, doneDate = 0,
       startDate = 0, activityDate = 0, idleSecs = 0, secondsDownloading = 0,
       secondsSeeding = 0, finished = false, queuePosition = 0,
       isStalled = false}, next = 0x7f6dec467b80, uniqueId = 71, bandwidth = {
       band = {{isLimited = false, honorParentLimits = true, bytesLeft = 0,
           desiredSpeed_Bps = 500000, raw = {newest = 5, transfers = {{date = 0,
                 size = 0}, {date = 1489195836133, size = 116}, {
                 date = 1489195836714, size = 105}, {date = 1489195836985,
                 size = 314}, {date = 1489195838143, size = 98}, {
                 date = 1489195865149, size = 96}, {date = 0, size = 0}, {
                 date = 0, size = 0}, {date = 0, size = 0}, {date = 0,
                 size = 0}}, cache_time = 0, cache_val = 0}, piece = {newest = 0,
             transfers = {{date = 0, size = 0}, {date = 0, size = 0}, {date = 0,
                 size = 0}, {date = 0, size = 0}, {date = 0, size = 0}, {
                 date = 0, size = 0}, {date = 0, size = 0}, {date = 0, size = 0},
               {date = 0, size = 0}, {date = 0, size = 0}}, cache_time = 0,
             cache_val = 0}}, {isLimited = false, honorParentLimits = true,
           bytesLeft = 0, desiredSpeed_Bps = 50000, raw = {newest = 5,
             transfers = {{date = 0, size = 0}, {date = 1489195836133,
                 size = 417}, {date = 1489195836714, size = 264}, {
                 date = 1489195837462, size = 96}, {date = 1489195838143,
                 size = 146}, {date = 1489195865149, size = 96}, {date = 0,
                 size = 0}, {date = 0, size = 0}, {date = 0, size = 0}, {
                 date = 0, size = 0}}, cache_time = 0, cache_val = 0}, piece = {
             newest = 0, transfers = {{date = 0, size = 0}, {date = 0, size = 0},
               {date = 0, size = 0}, {date = 0, size = 0}, {date = 0, size = 0}, {
                 date = 0, size = 0}, {date = 0, size = 0}, {date = 0, size = 0},
               {date = 0, size = 0}, {date = 0, size = 0}}, cache_time = 0,
             cache_val = 0}}}, parent = 0x560e62f8ab28, priority = 0 '\000',
       magicNumber = 43143, uniqueKey = 71, session = 0x560e62f8a920, children = {
         items = 0x7f6deca9c410, n_items = 0, n_alloc = 32}, peer = 0x0},
     swarm = 0x7f6dec46f570, desiredRatio = 2,
     ratioLimitMode = TR_RATIOLIMIT_GLOBAL, idleLimitMinutes = 30,
     idleLimitMode = TR_IDLELIMIT_GLOBAL, finishedSeedingByIdle = false}
   

   This tr_tier is the 52nd tier of its torrent:

   (gdb) p &(((tr_tier *) ((tr_ptrArray *)0x7f6df2813b60)->items[1])->tor)->tiers->tiers[51]
   $17 = (tr_tier *) 0x7f6dec4fb590
   (gdb) p (tr_tier *) ((tr_ptrArray *) 0x7f6df2813b60)->items[1]
   $18 = (tr_tier *) 0x7f6dec4fb590
   

   This tr_tier has two trackers:

   (gdb) p *((tr_tier *) ((tr_ptrArray *) 0x7f6df2813b60)->items[1])
   $19 = {byteCounts = {0, 0, 0}, trackers = 0x7f6dec45ed40, tracker_count = 2,
     currentTracker = 0x7f6dec45ed78, currentTrackerIndex = 1,
     tor = 0x7f6dec46fa10, scrapeAt = 1489195810,
     lastScrapeStartTime = 1489196040, lastScrapeTime = 0,
     lastScrapeSucceeded = false, lastScrapeTimedOut = false,
     announceAt = 1489196040, manualAnnounceAllowedAt = 1489196160,
     lastAnnounceStartTime = 0, lastAnnounceTime = 1489196040,
     lastAnnounceSucceeded = false, lastAnnounceTimedOut = false,
     announce_events = 0x7f6dec4614a0, announce_event_count = 1,
     announce_event_alloc = 4, key = 708, scrapeIntervalSec = 1800,
     announceIntervalSec = 600, announceMinIntervalSec = 120,
     lastAnnouncePeerCount = 0, isRunning = false, isAnnouncing = false,
     isScraping = true, wasCopied = false,
     lastAnnounceStr = "Connection failed", '\000' <repeats 110 times>,
     lastScrapeStr = '\000' <repeats 127 times>}
   

   The first tracker is:

   (gdb) p ((tr_tier *) ((tr_ptrArray *) 0x7f6df2813b60)->items[1])->trackers[0]
   $21 = {key = 0x7f6dec4f5f50 "udp://354.354.354.354:75535",
     announce = 0x7f6dec4f5a80 "udp://354.354.354.354:75535/annonuce",
     scrape = 0x7f6dec4f5ab0 "udp://354.354.354.354:75535/annonuce",
     tracker_id_str = 0x0, seederCount = -1, leecherCount = -1,
     downloadCount = -1, downloaderCount = 0, consecutiveFailures = 1, id = 60}
   

   Note that the scrape URL of this tracker is available. The second tracker is:

   (gdb) p ((tr_tier *) ((tr_ptrArray *) 0x7f6df2813b60)->items[1])->trackers[1]
   $22 = {key = 0x7f6dec4f5ba0 "http://354.354.354.354:75535",
     announce = 0x7f6dec4f5ae0 "http://354.354.354.354:75535/annonuce",
     scrape = 0x0, tracker_id_str = 0x0, seederCount = -1, leecherCount = -1,
     downloadCount = -1, downloaderCount = 0, consecutiveFailures = 0, id = 59}
   

   Note that the scrape URL of this tracker is NULL(0x0).

   The current tracker is pointed to trackers[1]:

   (gdb) p *(((tr_tier *) ((tr_ptrArray *) 0x7f6df2813b60)->items[1])->currentTracker)
   $24 = {key = 0x7f6dec4f5ba0 "http://354.354.354.354:75535",
     announce = 0x7f6dec4f5ae0 "http://354.354.354.354:75535/annonuce",
     scrape = 0x0, tracker_id_str = 0x0, seederCount = -1, leecherCount = -1,
     downloadCount = -1, downloaderCount = 0, consecutiveFailures = 0, id = 59}
   

4. 11 segfaults all happened at 0 ip xxxa0c sp xxxaf8 error 4 in libc-2.24.so[xxx000+195000]. 6 core files correspond to 3 different torrents. The tr_tier all have two trackers; one is udp://, the other is http://. All trackers are unable to connect.

5. If it is needed, I can provide core files via private email. And I have a debug logfile logged using "transmission-daemon --logfile /home/kralcyor/tmp/transmission.log --log-debug" for segfault of core.4; though it seems useless. If it is needed, I can also provide this logfile via private email.

core.4.backtrace-log.txt

[cfpp2p]

3a17304

[sandrotosi]

oooh that was fixed several months ago, maybe it's time to release a new version of transmission? :)

[kralcyor]

I am the original reporter.

I have not applied the commit(as it is rejected and need many modifications to make it works), but I think it seems that the commit will not fix.

The request->url is NULL(0x0); this causes memcmp() to segfault and is likely also cause strncmp() to segfault.

[cfpp2p]

@kralcyor Yes, I'm looking at the ramifications of changes to announcer.c from 2321bc3#diff-fd407c43b2a26523e0a5ebd68cc60a14 something about this particular commit doesn't look right to me as I look at it right now, but I'm not sure. I'll do some more analysis.

[cfpp2p]

The only way I might see tier->currentTracker->scrape inadvertently set to NULL
after passing through tierNeedsToScrape() might be from tr_announcerResetTorrent().
As perhaps a repeated automated script accessing add or replace trackers and resulting tr_torrentSetAnnounceList().

[kralcyor]

It seems that I do not use any script.

I guess what happened may like:

tier->currentTracker is pointed to "udp://" which has a non-NULL scrape URL.
The scrape URL is passed tierNeedsToScrape() check and the tier is appended to scrapeMe.(announcer.c:1538)

tier->currentTracker is annouced by tierAnnounce() and failed.(announcer.c:1554)
tierIncrementTracker() is called by on_announce_error();
this cause tier->currentTracker point to "http://" which has a NULL scrape URL.

Now tier->currentTracker is scraped by multiscrape() and cause segfault.(announcer.c:1558)

The attachments are new logs run by:

$ ulimit -c unlimited
$ export TR_DEBUG_FD=2
$ > ~/tmp/transmission.log ; transmission-daemon -f --logfile ~/tmp/transmission.log --log-debug 2> ~/tmp/transmission.runlog
$ cat ~/tmp/transmission.log | grep announcer.c | grep -C 20 "2017-05-29 19:34:52.057" > ~/tmp/transmission.log-part2
$ cat ~/tmp/transmission.runlog | grep announcer.c | grep -C 200 "19:34:51.683" > ~/tmp/transmission.runlog-part2

Some text changed.
I have applied a custom patch(attached) to avoid segfault.

The questioned torrents are NULL_FILENAME_1 and NULL_FILENAME_2, and tracker are udp://NULl_IP_PORT and http://NULl_IP_PORT.

Segfault is about to happen at transmission.log-part2:42-3.
The time is slightly different between the two logs.
In transmission.log-part2, the time is 19:34:52.057; in transmission.runlog-part2, the time is 19:34:51.684.

From transmission.runlog-part2:211-212(corresponding to transmission.log-part2:26), it is clear that udp://NULl_IP_PORT is fail to announce then tier->currentTracker is changed to http://NULl_IP_PORT.

transmission-log.txt
transmission-runlog.txt

avoid_request_null_scrape-patch.txt

[kralcyor]

Please note that some strings containing NULL_FILENAME_1 is truncated and may have no URL as NULL_FILENAME_1 is very long.

[cfpp2p]

@kralcyor Thanks for the information. I don't see that bug TRAC-6127 affects this situation but I think I see evidence of it in your logs.Could you please try the below patch and inform as to results with your situation. I'm continuing to investigate based on your analysis above. Thanks.

    /* send the requests we just built */
    for (int i = 0; i < request_count; ++i)
    {
        --announcer->slotsAvailable;
        scrape_request_delegate(announcer, &requests[i], on_scrape_done, announcer->session);
    }

ref: cfpp2p/transmission@83bca0a

[kralcyor]

@cfpp2p After applied the patch, still have the problem.

[kralcyor]

I have found a way to reproduce this issue in a fresh debootstrap Debian environment. Hope it helps.
And as now they contain no private informations, I can provide whole, untouched logs and core files if they are needed.

Way to reproduce this issue:
0.1 Set up udp://127.0.0.1:23810 and http://127.0.0.1:23810 as unreachable trackers
It seems that transmission-daemon will not segfault without these two commands.

$ sudo iptables -I INPUT -p udp --dport 23810 -j DROP 
$ sudo iptables -I INPUT -p tcp --dport 23810 -j DROP 

0.2 Set up debootstrap chroot

$ sudo debootstrap --variant=minbase sid sid/ http://ftp.debian.org/debian 
$ sudo mount -t proc proc sid/proc 
$ sudo mount -t sysfs sys sid/sys 
$ sudo mount -o bind /dev sid/dev 
$ sudo mount -t devpts pts sid/dev/pts 
$ sudo chroot sid/ su - 

Now as root in chroot.

# apt install transmission-daemon transmission-cli 
# adduser trans 
# exit

1. Run transmission-daemon
   $ sudo chroot sid su - trans
   Now as trans in chroot.

$ ulimit -c unlimited 
$ transmission-daemon --logfile ~/transmission.log
$ mkdir Downloads 
$ cd Downloads/
$ for i in {0..99} 
> do 
> dd if=/dev/urandom of=file${i} bs=1M count=1 
> transmission-create -o file${i}.torrent -t udp://127.0.0.1:23810 -t http://127.0.0.1:23810 file${i} 
> transmission-remote --add file${i}.torrent 
> sleep 0.4 
> done

transmission-daemon usually will segfault in 15 minutes after this point.

[cfpp2p]

I'm not able to trigger a call to multiscrape() with tierIncrementTracker() preceding. Perhaps the issue might be a socket and timeout problem related to threading.

@kralcyor What happens if you use REJECT instead of DROP with iptables? Try with 47, 48 or 49 torrents (instead of 100 torrents) or more or less until finding a trigger point number of torrents for the segfault.

    /* how many web tasks we allow at one time */
    MAX_CONCURRENT_TASKS = 48,

[mikedld]

The way I see it, HTTP tracker's tr_tracker_info::scrape is NULL right after metainfo loading since that is what tr_convertAnnounceToScrape (metainfo.c) returns: URL doesn't end in "/announce" (path is "/annonuce" in the original post unless it's an OPs mistake, and "/" in latter repro steps) and doesn't start with "udp:" either. Initializing tr_tracker::scrape from that uses tr_strdup which returns NULL as well.

What's interesting is how we end up scraping this tracker, as announceMore (announcer.c) explicitly checks (in tierNeedsToScrape) for scrape to be non-NULL before adding the item to scrapeMe array which then gets passed to multiscrape and then scrape_request_delegate. My current thinking is that it has to do with threading as well: tr_tier::currentTracker pointer changes between calls to tierNeedsToScrape (where it points to UDP tracker which has valid scrape URL and thus tier is added to scrapeMe array) and multiscrape (where it now points to HTTP tracker with NULL scrape URL).

[cfpp2p]

My current thinking is that it has to do with threading as well: tr_tier::currentTracker pointer changes between calls to tierNeedsToScrape (where it points to UDP tracker which has valid scrape URL and thus tier is added to scrapeMe array) and multiscrape (where it now points to HTTP tracker with NULL scrape URL).

@mikedld opinion?
@kralcyor could you test this?

static void announceMore(tr_announcer* announcer)
{
    int n;
    tr_torrent* tor;
    tr_ptrArray announceMe = TR_PTR_ARRAY_INIT;
    tr_ptrArray scrapeMe = TR_PTR_ARRAY_INIT;
    time_t const now = tr_time();

    dbgmsg(NULL, "announceMore: slotsAvailable is %d", announcer->slotsAvailable);

    if (announcer->slotsAvailable < 1)
    {
        return;
    }

    /* build a list of tiers that need to be announced */
    tor = NULL;

    while ((tor = tr_torrentNext(announcer->session, tor)) != NULL)
    {
        struct tr_torrent_tiers* tt = tor->tiers;

        for (int i = 0; tt != NULL && i < tt->tier_count; ++i)
        {
            tr_tier* tier = &tt->tiers[i];

            if (tierNeedsToAnnounce(tier, now))
            {
                tr_ptrArrayAppend(&announceMe, tier);
            }
        }
    }

    n = tr_ptrArraySize(&announceMe);

    /* if there are more tiers than slots available, prioritize */
    if (n > announcer->slotsAvailable)
    {
        qsort(tr_ptrArrayBase(&announceMe), n, sizeof(tr_tier*), compareTiers);
        n = announcer->slotsAvailable;
    }

    /* announce some */
    for (int i = 0; i < n; ++i)
    {
        tr_tier* tier = tr_ptrArrayNth(&announceMe, i);
        tr_logAddTorDbg(tier->tor, "%s", "Announcing to tracker");
        dbgmsg(tier, "announcing tier %d of %d", i, n);
        tierAnnounce(announcer, tier);
    }

    /* build a list of tiers that need to be scraped */
    tor = NULL;

    while ((tor = tr_torrentNext(announcer->session, tor)) != NULL)
    {
        struct tr_torrent_tiers* tt = tor->tiers;

        for (int i = 0; tt != NULL && i < tt->tier_count; ++i)
        {
            tr_tier* tier = &tt->tiers[i];

            if (!tierNeedsToAnnounce(tier, now) && tierNeedsToScrape(tier, now))
            {
                tr_ptrArrayAppend(&scrapeMe, tier);
            }
        }
    }

    /* scrape some */
    multiscrape(announcer, &scrapeMe);

    /* cleanup */
    tr_ptrArrayDestruct(&scrapeMe, NULL);
    tr_ptrArrayDestruct(&announceMe, NULL);
}

[kralcyor]

@cfpp2p after applied that, no this issue in several hours; it seems that indeed works. Thanks!

[mikedld]

I see what you're doing there @cfpp2p (moving scrapeMe population after tierAnnounce calls which might call back with error right away), and I agree this should help. Don't really like the double scan through the torrents list though, maybe this (below) will be better?

diff --git a/libtransmission/announcer.c b/libtransmission/announcer.c
index c44ec3148..0a8f58a9b 100644
--- a/libtransmission/announcer.c
+++ b/libtransmission/announcer.c
@@ -1474,6 +1474,8 @@ static void multiscrape(tr_announcer* announcer, tr_ptrArray* tiers)
         uint8_t const* hash = tier->tor->info.hash;
         bool found = false;
 
+        assert(url != NULL);
+
         /* if there's a request with this scrape URL and a free slot, use it */
         for (int j = 0; !found && j < request_count; ++j)
         {
@@ -1614,6 +1616,16 @@ static void announceMore(tr_announcer* announcer)
         tierAnnounce(announcer, tier);
     }
 
+    for (int i = tr_ptrArraySize(&scrapeMe); i > 0; --i)
+    {
+        tr_tier* tier = tr_ptrArrayNth(&scrapeMe, i - 1);
+
+        if (!tierNeedsToScrape(tier, now))
+        {
+            tr_ptrArrayRemove(&scrapeMe, i - 1);
+        }
+    }
+
     /* scrape some */
     multiscrape(announcer, &scrapeMe);
 

Or we could always just skip tiers with NULL URL in multiscrape...

[cfpp2p]

@mikedld
tierIncrementTracker() sets tr_tier::isScraping to false
and possibly allowing this tier to now scrape.

Repopulating scrapeMe after tierAnnounce() then could correctly add
that tier to scrape. Removing invalid scrapeMe array elements
should work however it wouldn't catch any newly allowed
( by tierIncrementTracker() ) tier to scrape.

That's the way I see it right now.

Or we could always just skip tiers with NULL URL in multiscrape...

As a failsafe we could tierNeedsToScrape() in multiscrape ;)