Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libtorrent 32-bit Windows build segfaults on partial resume_data #8353

Closed
kotenok2000 opened this issue Dec 23, 2024 · 12 comments · Fixed by #8412
Closed

libtorrent 32-bit Windows build segfaults on partial resume_data #8353

kotenok2000 opened this issue Dec 23, 2024 · 12 comments · Fixed by #8412
Assignees
Milestone

Comments

@kotenok2000
Copy link

Traceback and/or steps to reproduce
Tribler 8.0.7 crashes when i attempt to import a torrent with already existing downloaded files from an old version.
I pressed "Import torrent as a file". opened .torent file and set destination to F:\TriblerDownloads\TRON RUN-r [FitGirl Repack]\

Insert your traceback here!

CrashDumps.zip

@qstokkink
Copy link
Contributor

Thanks for the report! I haven't been able to reproduce this yet: if you have any more information on what triggers this crash, please let us know.

@qstokkink qstokkink added this to the 8.1.0 milestone Jan 6, 2025
@qstokkink
Copy link
Contributor

Alright, I tried just about every variation of importing old downloads. I corrupted the files (downloaded, conf, state, parts), added odd file names (spaces, special characters), etc.

I looked at your crashdumps again. Is it correct that you're using 32-bit Windows 10? I may be the case that one of our dependencies is secretly using 64-bit addressing and importing a torrent just happened to trigger it by accident.

@kotenok2000
Copy link
Author

kotenok2000 commented Jan 22, 2025

I am using 64 bit windows 11 but 32 bit tribler.

I have recreated the bug and it has created this file in dlcheckpoints.
Tribler crashes when it attempts to load it.

[Admin edit: purged the .conf file (thanks for helping with the reproduction!)]

@qstokkink
Copy link
Contributor

qstokkink commented Jan 22, 2025

Thanks, I'll try to reproduce it using that conf file. So far:

  • Ubuntu: works.
  • Windows x64 using x64 Tribler: works.
  • Windows x64 using x32 Tribler: crashes.
  • Windows x64 using x32 Tribler + removed engineresumedata from the .conf file: works.
  • Windows x64 using x32 Tribler + set hops to 0 in the .conf file: crashes.
  • Windows x64 using x32 Tribler + edited the engineresumedata dict in the .conf file to include the b"save_path": works.
  • Windows x64 using x32 bleeding edge Tribler (lt 2.0.9): crashes.

Zooming in on the engineresumedata/resume_data for 32 bit (❌ means SEGFAULT and ✅ means no crash):

  • ❌: b'file-format': b'libtorrent resume file'
  • ❌: b'file-version': 1
  • ❌: b'info-hash': ...OMITTED...
  • ✅: b'save_path': b'F:/BiglyBT Downloads/'
  • ❌: b'save_path': b''
  • ❌: b'file-format': b'libtorrent resume file', b'file-version': 1
  • ❌: b'file-format': b'libtorrent resume file', b'file-version': 1, b'info-hash': ...OMITTED...
  • ✅: b'file-format': b'libtorrent resume file', b'file-version': 1, b'info-hash': ...OMITTED..., b'save_path': b'F:/BiglyBT Downloads/'

@qstokkink qstokkink self-assigned this Jan 22, 2025
@qstokkink qstokkink changed the title Tribler crashes if download directory isn't empty. libtorrent 32-bit Windows build segfaults on partial resume_data Jan 24, 2025
@qstokkink
Copy link
Contributor

My conclusion is that the libtorrent (both 1.2.19 and 2.0.9) 32-bit Windows build crashes (i.e., tries to access a NULL-pointer and SEGFAULTS) if, and only if, the save_path in the resume data is either unset or an empty string (CC: @arvidn).

I guess it's fine to just make sure the b'save_path' is always added (idea by @egbertbouman), even though it's not necessary for any other build.

@arvidn
Copy link

arvidn commented Jan 24, 2025

yeah, save_path is currently treated as a required argument, and there's an assert to make sure it's passed in. It's an oversight that this is not handled well when loading (possibly unreliable) resume data from disk.
I'll take a look.

@arvidn
Copy link

arvidn commented Jan 24, 2025

does this address the issue? It turns the invalid add_torrent_params into an exception when added to the session.
arvidn/libtorrent#7825
arvidn/libtorrent#7826

@qstokkink
Copy link
Contributor

qstokkink commented Jan 25, 2025

@arvidn thanks! I'll check it out. I will have to make a setup to compile the 32-bit Windows Python wheel though, so it may take me a little while to verify. Please give me a few days.

Current custom build status:

@qstokkink
Copy link
Contributor

@arvidn I built a wheel from your arvidn/libtorrent#7826 PR and used it with our Tribler 32-bit build. The result is:

  • Tribler win32 now segfaults with and without a save_path in the resume_data?!
  • Adding a torrent (using add torrent parameters) without resume_data still works.

Looking at your PR, I can't quite make sense of that behaviour. Perhaps Tribler's resume data is very wrong? I'm using the following:

import libtorrent
resume_data = libtorrent.bencode({
    b'file-format': b'libtorrent resume file',
    b'file-version': 1,
    b'info-hash': ...,  # Omitted for privacy
    b'save_path': b'F:/BiglyBT Downloads/'  # Also attempted with backslashes, and no trailing slash
})

This used to work before (in 2.0.9) and no longer works:

import libtorrent
resume_data = libtorrent.bencode({b"save_path": b"F:\\BiglyBT Downloads\\"})

@qstokkink
Copy link
Contributor

qstokkink commented Jan 27, 2025

We're passing fast resume data in the atp. According to https://github.com/arvidn/libtorrent/blob/RC_2_0/include/libtorrent/add_torrent_params.hpp#L389-L395 and https://www.libtorrent.org/upgrade_to_1.2-ref.html#resume-data-handling, this is now deprecated. Maybe we just need to modernize our source code?

I still think it's somewhat odd that this only crashes Windows 32 bit.

@arvidn
Copy link

arvidn commented Jan 27, 2025

This used to work before (in 2.0.9) and no longer works:

import libtorrent
resume_data = libtorrent.bencode({b"save_path": b"F:\BiglyBT Downloads\"})

What happens instead? It seems to work on linux (it's not expected to segfault)

@qstokkink
Copy link
Contributor

@arvidn Indeed, this exact combination of atp + resume_data key works on my win64 build as well as Ubuntu (+ whatever linux you tested).

Only using the win32 build (on a x64 Windows 10/11 system) do I, and OP, see the 0xc0000005 Access Violation. From the crash dump, there seems to be a dereferenced null pointer somewhere.

I copied over your cibuildwheel GitHub Actions. I first thought that maybe the assertions were only enabled on win32 or that the exceptions were not switched on for win32. From what I found, neither seems to be the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants