frostwire / frostwire-jlibtorrent

A swig Java interface for libtorrent by the makers of FrostWire. Develop libtorrent based apps with the joy of coding in Java.
http://www.frostwire.com
MIT License
444 stars 137 forks source link

[desktop] Calling SessionManager stop() takes forever after torrent is finished #273

Closed pvishnyakov closed 2 years ago

pvishnyakov commented 2 years ago

Hi.

I’m using jlibtorrent 1.2.15.2 and I’ve found that I can’t stop the session manager after the torrent has reached the Finished state - calling stop() never returns. If I call SessionManager stop() while the torrent is still downloading, everything is ok.

Brief debugging through SessionManager stop() source code shows that everything hangs on s.delete() in this block:

            …

            resetState();

            s.delete();

            onAfterStop();

        } finally {
            sync.unlock();
        }

What is the proper way to stop the session, release all used resources and clean up everything after the torrent download is completed?

gubatron commented 2 years ago

Thank you, testing...

gubatron commented 2 years ago

Just tested on Android 12, jlibtorrent-1.2.15.2, debugger didn't freeze on s.delete()

What is the proper way to stop the session, release all used resources and clean up everything after the torrent download is completed?

You should only stop the session in case the user somehow wants to disconnect from the bittorrent network or the app is being shutdown.

Ideally you want to let the session manage the torrents downloads.

If the user hits "Stop" on a torrent, what we really do is we pause the torrent handle

public void pause() {
        if (!th.isValid()) {
            return;
        }
        extra.put(WAS_PAUSED_EXTRA_KEY, Boolean.TRUE.toString());
        // notice how we mark the transfer as non-automanaged by libtorrent
        // otherwise the torrent could be automatically unpaused to keep seeding
        th.unsetFlags(TorrentFlags.AUTO_MANAGED);
        th.pause();
        doResumeData(true);
}
pvishnyakov commented 2 years ago

Thanks. Actually this behavior was detected not in the Android, it was Java app (Java 8), but I remember in the Android 9 I’ve met something similar with the older version of Jlibtorrent.

About the reason - in my case I want to stop the session and clean up everything when I’m seeding the file and want to replace this file with new version. Since I’m always seeding, the download finishes immediately and seeding continues until the new version of the file is available. New file check is performed every hour. This app should run unattended for months, if I just pause TorrentHandle and open new one for seeding - what will happen with the previous paused handlers and resources occupied by them?

gubatron commented 2 years ago

Thanks. Actually this behavior was detected not in the Android, it was Java app (Java 8), but I remember in the Android 9 I’ve met something similar with the older version of Jlibtorrent.

thank you for bringing this up.

This reminds me that we've had this weird freeze on shutdown on some Windows instances, and this might be the culprit. I'll try to replicate on Windows and see if it happens.

However, I will not be testing with Java 8, I'll be testing with Java 17.0.1 which is bundled with our app.

About the reason - in my case I want to stop the session and clean up everything when I’m seeding the file and want to replace this file with new version.

I don't understand what you mean by "replace this file with new version". If you have a new version of a file, then that means you have a completely different torrent. You don't need to stop the session, you just need to finish that transfer, remove it from the session and add a new transfer to the session with the new version. But then again, I think I'm misunderstanding what you mean.

Since I’m always seeding, the download finishes immediately and seeding continues until the new version of the file is available. New file check is performed every hour.

Not sure what you mean. Please explain. Is this some sort of dynamic torrent that you have? I guess you have a very particular use-case.

And I agree the app should be running unattended for years :)

pvishnyakov commented 2 years ago

I don't understand what you mean by "replace this file with new version". If you have a new version of a file, then that means you have a completely different torrent. … Not sure what you mean. Please explain. Is this some sort of dynamic torrent that you have?

Actually it’s all simple. Once started, my app is downloading the file from cloud storage and start seeding it. Every hour it’s checking the cloud storage again and again to see if new version of the file is available. If new version found - it’s downloaded and previous file (that is currently seeding) must be removed from session and new torrent is starting. To avoid wasting the resources I was trying to completely stop and re-create the session for every new seed.

After this freezing issue I’m doing as you recommended - delete the torrent from the session and just add new torrent to the existing session. To kill old unused handles I’m restarting the process every 4 hours. Not the best solution but it works. If you can eliminate this issue it will improve end-user experience because currently the download may stop in the middle if the process restart and file version update will happen at the same time.

gubatron commented 2 years ago

Update: .stop() works fine on Mac Desktop.

Will test on Windows next.

gubatron commented 2 years ago

Got it, I wouldn't stop the session, simply remove old torrent handle and create a new torrent with the newly downloaded file.

Shutting down and restarting a session is a delicate process that IMHO is unnecessary for your use case.

If you want to do that you must properly save the session state and recover it again, all for nothing since your app is still running.

You want to shutdown usually only when the app is being closed for good.

gubatron commented 2 years ago

Final update:

I've now also tested and stepped debug over the shutdown process on SessionManager.stop() and stepped into s.delete() and could not replicate any freeze in Windows 10.

My guess is that how you were stopping and restarting the session to start a new transfer was perhaps creating a race condition.

Will close ticket as I can't find a freeze or bug.

Try using a single session during the lifetime of the process.