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

Adding a download with changed priorities doesn't recognize the new priorities if download was finished earlier. #269

Closed pratik-pattanaik closed 2 years ago

pratik-pattanaik commented 2 years ago

Adding a torrent using SessionManager.download(TorrentInfo ti, File saveDir, File resumeFile, Priority[] priorities, List peers) doesn't recognize the changed file selections if the torrent was in a finished state in the previous session. It starts with FINISHED state.

This happens in 1.2.15.1. Used to work perfectly in 1.2.0.6, returning a DOWNLOADING state instead of finished state. Reproducible in Android 11 arm64.

gubatron commented 2 years ago

if the torrent was in a finished state in the previous session. It starts with FINISHED state.

That sounds to me like it's resuming correctly. It was finished, therefore you restore it in the same state.

Is the torrent managed by libtorrent? perhaps you have too many torrents in active state and libtorrent decides not to resume it.

gubatron commented 2 years ago

This is how we resume downloads on FrostWire, and we don't have the issue you're having with 1.2.15.1

    public void resume() {
        if (!th.isValid()) { //th as in torrent handle
            return;
        }
        extra.put(WAS_PAUSED_EXTRA_KEY, Boolean.FALSE.toString());
        th.setFlags(TorrentFlags.AUTO_MANAGED);
        th.resume();
        doResumeData(true);
    }

    private void doResumeData(boolean force) {
        long now = System.currentTimeMillis();
        if (force || (now - lastSaveResumeTime) >= SAVE_RESUME_RESOLUTION_MILLIS) {
            lastSaveResumeTime = now;
        } else {
            // skip, too fast, see SAVE_RESUME_RESOLUTION_MILLIS
            return;
        }
        try {
            if (th != null && th.isValid()) {
                th.saveResumeData(TorrentHandle.SAVE_INFO_DICT);
            }
        } catch (Throwable e) {
            LOG.warn("Error triggering resume data", e);
        }
    }    
pratik-pattanaik commented 2 years ago

if the torrent was in a finished state in the previous session. It starts with FINISHED state.

That sounds to me like it's resuming correctly. It was finished, therefore you restore it in the same state.

Is the torrent managed by libtorrent? perhaps you have too many torrents in active state and libtorrent decides not to resume it.

So, the torrent was in a finished state, but then the file priorities were changed to include more files to download, which means that the torrent should no longer be in the finished state but the downloading state. This used to happen back in 1.2.0.6, but it's not working that way now.

pratik-pattanaik commented 2 years ago

Hi,

Any updates here ?

gubatron commented 2 years ago

Hi Pratik.

I just performed the following test:

  1. Downloaded a single file on a torrent with many files.
  2. The torrent went to seeding (as I have "seeding" settings on our app on)
  3. Started a torrent download again with another set of files in the torrent.
  4. Torrent finished downloading the new files and went to seeding.

Then I turned off seeding, the torrent went to "Finished" state.

  1. Started downloading another set of files in the torrent
  2. Downloads started as expected
  3. Downloads finished.
  4. Torrent went back to finished state.

Works as expected, check the logic in your code when starting a partial download.

gubatron commented 2 years ago

Hoping it helps, this is how we use jlibtorrent to start a partial download in FrostWire

public void download(File torrent, File saveDir, boolean[] selection) {
        if (swig() == null) {
            return;
        }
        saveDir = setupSaveDir(saveDir);
        if (saveDir == null) {
            return;
        }
        TorrentInfo ti = new TorrentInfo(torrent);
        if (selection == null) {
            selection = new boolean[ti.numFiles()];
            Arrays.fill(selection, true);
        }
        Priority[] priorities = null;
        TorrentHandle th = find(ti.infoHash());
        boolean exists = th != null;
        if (selection != null) {
            if (th != null) {
                priorities = th.filePriorities();
            } else {
                priorities = Priority.array(Priority.IGNORE, ti.numFiles());
            }
            boolean changed = false;
            for (int i = 0; i < selection.length; i++) {
                if (selection[i] && priorities[i] == Priority.IGNORE) {
                    priorities[i] = Priority.NORMAL;
                    changed = true;
                }
            }
            if (!changed) { // nothing to do                                                                                                                                                 
                return;
            }
        }
        download(ti, saveDir, priorities, null, null);
        if (!exists) {
            saveResumeTorrent(ti);
        }
    }

now that second download(ti,saveDir, priorities, null, null); method should be of interest, look how we resume:

private void download(TorrentInfo ti, File saveDir, Priority[] priorities, File resumeFile, List<TcpEndpoint> peers) {
        TorrentHandle th = find(ti.infoHash());
        if (th != null) {
            // found a download with the same hash, just adjust the priorities if needed                                                                                                     
            if (priorities != null) {
                if (ti.numFiles() != priorities.length) {
                    throw new IllegalArgumentException("The priorities length should be equals to the number of files");
                }
                th.prioritizeFiles(priorities); // perhaps you're missing this call.
                fireDownloadUpdate(th);
                th.resume();
            } else {
        // did they just add the entire torrent (therefore not selecting any priorities)                                                                                             
                final Priority[] wholeTorrentPriorities = Priority.array(Priority.NORMAL, ti.numFiles());
                th.prioritizeFiles(wholeTorrentPriorities);
                fireDownloadUpdate(th);
                th.resume();
            }
        } else { // new download                                                                                                                                                             
            download(ti, saveDir, resumeFile, priorities, peers);
            th = find(ti.infoHash());
            if (th != null) {
                fireDownloadUpdate(th);
            }
        }
    }

I think you might be missing a call to th.prioritizeFiles(priorities);

pratik-pattanaik commented 2 years ago

Hi Angel,

I'll list down the steps to reproduce the issue (sorry, should have made it clearer from the start).

  1. Add and start a download with many files.
  2. After download starts, unselect some of the files and wait for download to finish.
  3. Force close the app and restart it. A fresh session is started and the torrent is not yet added to this session.
  4. Select a file that had been unselected earlier.
  5. Start the download.

Result : Download begins with FINISHED state and doesn't download the re-selected files.

However, if you omit step 3 from above, that is, when the torrent handle can be found in current session, the change in priorities is recognized and the download proceeds normally. Only when th == null & you reach download(ti, saveDir, resumeFile, priorities, peers); of the 2nd method does this issue occur.

gubatron commented 2 years ago

Force close the app and restart it

Seems to me that this might be related to your issue. You might not be saving the state of the torrent during a force close.

A fresh session is started and the torrent is not yet added to this session.

This confirms it, you should be able to recover the previous session and the state of the torrent. If you're not saving the session state anywhere this is really an issue with your app architecture, not jlibtorrent/libtorrent.

If you start downloading a torrent that you already downloaded, libtorrent will find that the pieces already exist and it doesn't need to download them again (that would be inefficient on the network, there's no need to download again what you have already downloaded and hashes out correctly), this is why it doesn't download again.

If you're concerned about a user force closing half way, you should periodically save the state of the libtorrent session in case that happens.

pratik-pattanaik commented 2 years ago

So, the torrent state was saved prior to force close when it was in the FINISHED state. After force close and restart, in Step 4, we change the file selection to include more files and add the torrent back again for download. Yet, it starts with FINISHED state only.

The saving part is not really the issue here, since we're adding more files after the force close. So, even if the state wasn't saved in the previous session, it should recognize that the download isn't complete as per the latest selection of files in the new session.