Should add timeout for udp trackers in PeriodicalAnnouncer about rain HOT 7 CLOSED

Yes, i'm experience a hang larger than TrackerStopTimeout.

It's not the stopping torrent's PerioidAnnouncer that hangs, it's other running torrents's PerioidAnnouncer hang because of udp tracker.

All announcers share the same udptracker.Transport in TrackerManager

type TrackerManager struct {
	httpTransport *http.Transport
	udpTransport  *udptracker.Transport
}

And there is one lock in udptracker.Transport

func (t *Transport) getConnection(addr string) *connection {
	t.m.Lock()
	defer t.m.Unlock()
	conn, ok := t.connections[addr]
	if !ok {
		conn = new(connection)
		t.connections[addr] = conn
	}
	return conn
}

So when we get lock and hang in Transport.retryTransaction, then another torrent call Transport.getConnection will hang too.

func (t *Transport) retryTransaction(ctx context.Context, f func(*transaction), trx *transaction) ([]byte, error) {
         // this hang until udp tracker done
        // before that, all other torrents call getConnection will hang because of this lock
	t.m.Lock()
	t.transactions[trx.ID()] = trx
	t.m.Unlock()
...
}

And yes, the stopping torrent's announcer.NewStopAnnouncer will hang for a long time, that's what i'm experiencing

Maybe i'm wrong, but please check anyway, thanks.

from rain.

I think I get this. I have a solution in my mind. I'll try to implement this weekend.

from rain.

When stopping a torrent, in order to prevent this scenario, first, PerioidAnnouncers are stopped. This operation does not block.

The torrent goes into Stopping state.

Then, another Announcer is started that will only announce one time, with a timeout of TrackerStopTimeout which is 5 seconds by default (but configurable via config file). This is required to tell the tracker that we are stopping the torrent and tell it to remove our client from the peer list.

This still does not hang. If you don't want to wait for last announce, you can call Torrent.Stop() again to stop it immediately.

Do you experience a hang larger than TrackerStopTimeout?

from rain.

I think I get this. I have a solution in my mind. I'll try to implement this weekend.

Hi @cenkalti ，is there any progress?

from rain.

Sorry, I didn't have enough time. I'll try to look again this weekend.

from rain.

Hey @kylege . This issue turned out to be a larger refactor rather that a small fix. I removed the lock completely and replaced it with a goroutine in UDP transport. I already merged it after testing on my own seedbox but feel free to comment and ask questions on the commit: 3321278

Please let me know what do you think. Cheers!

from rain.

Hey @kylege . This issue turned out to be a larger refactor rather that a small fix. I removed the lock completely and replaced it with a goroutine in UDP transport. I already merged it after testing on my own seedbox but feel free to comment and ask questions on the commit: 3321278

Please let me know what do you think. Cheers!

Nice work! Problem solved perfectly.

from rain.