webseed slow to stop problem

[cfpp2p]

webseed torrents can be very slow to pause/stop which can lead to problems if you choose
'Trash Data & Remove From List' on an actively downloading webseed torrent(s).


It has been months since I've looked at this issue but I do remember that in about 1 out of 25
'Trash Data & Remove From List' I would get a crash.

If you pause/stop actively downloading webseed torrent(s) many times you will see the torrent(s) in a stopped/paused state with a high downloading transfer rate for a long time after (from like 5 seconds to a minute or so).
see:
https://trac.transmissionbt.com/ticket/4576#comment:13
and
https://trac.transmissionbt.com/ticket/4644#comment:19

Here is the patch that I have been using for a long time and have not gotten a crash since using it. The patch won't allow
'Trash Data & Remove From List' on running torrent(s) with webseed. With this patch 'Trash Data & Remove From List' will only pause/stop running webseed torrents, then once stopped choose again 'Trash Data & Remove From List'. Non webseed torrents behavior is not changed. Already paused/stopped torrents behavior is not changed.

Code: Select all

--- J:/!/WebSeedMagDbFree/pcTMP-&-regular-Work/svn/rpcimpl.c.13504.c	Fri Nov  2 11:05:23 2012
+++ J:/!/WebSeedMagDbFree/pcTMP-&-regular-Work/svn/rpcimpl.c	Fri Nov  2 11:17:39 2012
@@ -345,9 +345,22 @@
     for( i=0; i<torrentCount; ++i )
     {
         tr_torrent * tor = torrents[i];
+
+        if( tor->isRunning && ( tor->info.webseedCount > 0 ) )
+        /* disable removing actively running torrents with webseeds until
+        bandwidth-pulse stops the transfer first
+        fixes crash of improperly interupting peer-io callback call chain
+        SRS -- https://trac.transmissionbt.com/ticket/4576#comment:13 */
+        {
+            tor->isStopping = true;
+            notify( session, TR_RPC_TORRENT_STOPPED, tor );
+        }
+
+        else {
         const tr_rpc_callback_status status = notify( session, type, tor );
         if( !( status & TR_RPC_NOREMOVE ) )
             tr_torrentRemove( tor, deleteFlag, NULL );
+        }
     }
 
     tr_free( torrents );

https://trac.transmissionbt.com/ticket/5069 and https://trac.transmissionbt.com/ticket/5063 have addressed some issues pertaining to webseeds with magnetlinks however the slow to pause/stop problems have nothing to do with magnetlinks.

[cfpp2p]

It won't work just to add just add a tr_torrentStop to tr_torrentRemove because as is shown by rpcimpl.c code for torrentStop()

Code: Select all

static const char*
torrentStop( tr_session               * session,
             tr_benc                  * args_in,
             tr_benc                  * args_out UNUSED,
             struct tr_rpc_idle_data  * idle_data UNUSED )
{
    int           i, torrentCount;
    tr_torrent ** torrents = getTorrents( session, args_in, &torrentCount );

    assert( idle_data == NULL );

    for( i = 0; i < torrentCount; ++i )
    {
        tr_torrent * tor = torrents[i];

        if( tor->isRunning || tr_torrentIsQueued( tor ) )
        {
            tor->isStopping = true;
            notify( session, TR_RPC_TORRENT_STOPPED, tor );
        }
    }
    tr_free( torrents );
    return NULL;
}

isStopping must be used since the webseed code doesn't immediately allow the torrent to stop properly. From bandwidthPulse() code:

Code: Select all

        /* stop torrents that are ready to stop, but couldn't be stopped
           earlier during the peer-io callback call chain */
        if( tor->isStopping )
            tr_torrentStop( tor );

and that is the reason to use isStopping for the webseed code. We must wait for bandwidthPulse. The webseed code may fault and crash if the peer-io callback call chain is simply cut off by tr_torrentStop instaed of using the isStopping flag. Adding tr_torrentStop to tr_torrentRemove would change the way tr_torrentRemove functions application wide and besides torrent.c stopTorrent() is already in the path of tr_torrentRemove and also in the path of tr_torrentStop so we'd be calling it unnecessarily twice.

The issue and the work-around patch that I've posted that prevents the crash has been tested for transmission-remote, RPC and web-client. I don't know how the Mac client would handle a torrent that is stopped but continues downloading in the stopped state. This only happens with a connected webseed.

[cfpp2p]

If you are getting crashes, shouldn't a trac ticket be opened?

As I remember it took many, many attempts to get the crash to occur. I had to have a particular webseed and it also seemed to depend on the other torrents that were concurrently uploading/downloading. Once I did find the correct combination I could crash transmission about 1 out of 25 times. This all came about sometime after (month or two?)
webseed downloading never gets downloaded https://trac.transmissionbt.com/ticket/4666 when I was doing my final webseed testing.

If someone wants to try a couple hundred times of 'Trash Data & Remove From List' on different webseed torrents, with all the variations of PEX, uTP, DHT and doing so at just the right time I would pretty much guarantee they could crash transmission. Also it probably depends on the server of the webseed and what traffic it is experiencing along with whether it's okay to gzip the response, or deflate it, or leave it raw. Then maybe a ticket with the appropriate information could be constructed.

The crash is extremely obscure but I know it does occur. It seems to me the problem (webseed slow to stop problem)/(crash--on--trash) occurs because of webseed slow to stop problem and I'm confident that my patch
viewtopic.php?f=1&t=14053#p62388
will prevent the crash. My patch does make a minor (in my opinion) inconvenience of having to use trash data twice on webseed torrents (or using stop/pause first then trash) but I consider crashing transmission not an option ever. Just testing current trunk with a few different webseeds and about 20 or so tries of trash data I could not crash transmission.

I don't consider the fact that webseeds stop so slowly correct but this is not a bug that I've ever crashed transmission with.

I've provided the patch viewtopic.php?f=1&t=14053#p62388
for anyone with the same concerns.