mirror of
https://github.com/qbittorrent/qBittorrent.git
synced 2026-03-02 22:57:32 -05:00
Crash with showing the downloading file per peer #3751
Labels
No labels
Accessibility
AppImage
Bounty
Build system
CI
Can't reproduce
Code cleanup
Confirmed bug
Confirmed bug
Core
Crash
Data loss
Discussion
Docker
Documentation
Duplicate
Feature
Feature request
Feature request
Feature request
Filters
Flatpak
GUI
Has workaround
I2P
Invalid
Libtorrent
Look and feel
Meta
NSIS
Network
Not an issue
OS: *BSD
OS: Linux
OS: Windows
OS: macOS
PPA
Performance
Project management
Proxy/VPN
Qt bugs
Qt6 compat
RSS
Search engine
Security
Temp folder
Themes
Translations
Triggers
Waiting diagnosis
Waiting info
Waiting upstream
Waiting web implementation
Watched folders
WebAPI
WebUI
autoCloseOldIssue
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/qBittorrent#3751
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @hdcTenBasePid on GitHub (Jan 19, 2016).
Ver - OpenVPN 2.3.10 , Tap 9.0.21.1
I an using advanced adapter setting to select VPN adapter.
Although a crash is reported, qBit continues even now as I type this. Here is your info:
qBittorrent version: v3.3.2
Libtorrent version: 1.0.8.0
Qt version: 5.5.1
Boost version: 1.60.0
@sledgehammer999 commented on GitHub (Jan 19, 2016):
Is this happening only if you use your VPN and not throught your normal connection? To be honest the above seems irrelevant to the connection method.
@sledgehammer999 commented on GitHub (Jan 19, 2016):
There are other duplicates so it isn't a VPN related issue. It seems to occur from this implemented feature: #2885. @evsh care to take a look?
In any case, I'll try to debug this later today.
@hdcTenBasePid commented on GitHub (Jan 19, 2016):
Hi, It hasn't happened just thought WiFi and has only occurred once today on Vpn.
Maybe, I'm a bit premature. Will follow up if can work it out.
Regs
Sent from RB's Windows Phone
From: sledgehammer999mailto:notifications@github.com
Sent: 19/01/2016 8:38 PM
To: qbittorrent/qBittorrentmailto:qBittorrent@noreply.github.com
Cc: hdcTenBasePidmailto:rboras10@hotmail.com.au
Subject: Re: [qBittorrent] Crash on 10586.63 downloading through VPN (#4597)
Is this happening only if you use your VPN and not throught your normal connection? To be honest the above seems irrelevant to the connection method.
Reply to this email directly or view it on GitHub:
https://github.com/qbittorrent/qBittorrent/issues/4597#issuecomment-172792968
@sledgehammer999 commented on GitHub (Jan 19, 2016):
Maybe this happens only with magnet links?
@zeule commented on GitHub (Jan 19, 2016):
@sledgehammer999 , sure thing. @hdcTenBasePid, what is the speed of your connection and what is the piece size of the torrent which lead to this crash?
@Nemo-qB commented on GitHub (Jan 19, 2016):
I think that I just got the same crash report, qBittorrent keeps running on the background while the crash report is showing. I had just installed it, started a new torrent and crashed within seconds. I can't seem to reproduce the crash.
@sledgehammer999 commented on GitHub (Jan 19, 2016):
This is a serious bug. I already closed another 7 duplicate reports.
@naikel commented on GitHub (Jan 19, 2016):
Definitely a serious bug. Looks like in this line:
s.file_index can be an invalid value for filePath.
I looked examples at this kind of code in libtorrent and in all cases before accesing file_path() they validate pad_file_at like this:
Because the "pad file" whatever that is doesn't have a file path or something like that.
@sledgehammer999 commented on GitHub (Jan 19, 2016):
@arvidn some clarification: Is the index of
torrent_info::file_path()andtorrent_info::files().file_path()exactly the same semantically? Or are you referring to 2 different things there?In case you can help here is the functions' code from the backtrace:
And the index passed to
BitTorrent::TorrentInfo::filesForPiece(pieceIndex)is obtained frompeer_info::downloading_piece_index@naikel commented on GitHub (Jan 19, 2016):
I'll elaborate a little more since just for the purposes of documentation. #2885 was implemented like this:
And a couple of examples in libtorrent of this kind of code:
Another:
I hope you can see what I mean...
@sledgehammer999 commented on GitHub (Jan 19, 2016):
@naikel good find. Let's wait a bit for @arvidn to maybe confirm yours if he's willing to take a look.
FYI, "padding files" is an invention of BitComet. It creates dummy files that take the remaining size of the last piece of a file, when that last piece will extend to the next file normally. It is a way to have aligned(?) files. Shitty invention that pollutes the filesystem with useless files.
If you're correct then the users have some torrent in their queue created by BitComet with padding files.
@Nemo-qB commented on GitHub (Jan 19, 2016):
Now you have changed the issue name; thats exactly where it has crashed. I was looking at the peers at that moment when it suddenly happened. I had only 1 torrent (total) running created by uTorrent.
@sledgehammer999 commented on GitHub (Jan 19, 2016):
Is it reproducible with that? Can you share it? (or email it to me sledgehammer999 (at) qbittorrent (dot) org)
@Nemo-qB commented on GitHub (Jan 19, 2016):
I will try to reproduce the crash, I will also try to reproduce it with different torrents. Give me a few minutes, will edit this post when im done.
Edit 1:
It seems that the new column sometimes shows empty space for a moment instead of the filename while downloading from the peer, not sure if this is intented like this (also not sure if it has something to do with the crash). The filename dissappears and comes back afterwards. The places with the red line are empty, see screenshot:
The same torrent that was chrashed is going good for 10 minutes. Weird. I will try a different torrent now.
Edit 2:
Second torrent didn't crashed also. Now I will try to find a torrent that is created by Bitcomet that has padding files (if I can find a healty one).
Edit 3:
It seems that qBittorrent doesn't download the Bitcomet padding files automaticly, ''No downloaded'' it says:
The torrent is created by Bitcomet 1.35, running for few minutes now without crash. Choosing the padding files to download shows them 100% right away.
I can't seem to reproduce this crash after my tests. I have to go to sleep now, will try to help further if needed.
@arvidn commented on GitHub (Jan 19, 2016):
@sledgehammer999 do you mean
torrent_info::file_at()? Yes, the index is the file index. which is in the range [0, num_files).speaking of padding files, I think they make a lot of sense. They only pollute the filesystem if your client doesn't support them (so they are backwards compatible) and if the creator of the torrent had poor quality (and places the padding files in places to they maximize their pollution, e.g. bitcomet). However, they enable features that should have been supported from the start. specifically:
The latter is important to the internet archive (archive.org).
@sledgehammer999 commented on GitHub (Jan 19, 2016):
Ok. I see your reasoning. Do you think pad files have something to do with our crashes? -I can't reproduce them-
@zeule commented on GitHub (Jan 19, 2016):
@arvidn , can we call
file_storage::file_path()with any index, returned bytorrent_info::file_at(), or should we exclude pad files?@arvidn commented on GitHub (Jan 19, 2016):
torrent_info::file_at()is a deprecated function that's meant to be replaced bytorrent_info::files().xxx()where xxx is individual accessors for various properties of a file.torrent_info::file_at()returns a struct with all properties of a file (that are kept internally). It doesn't return an index as far as I know. The reason for this change is that it gives the implementation a lot more freedom to store the information in more efficient ways.pad files also have indices assigned to them. the index is literally the index the file entry appears in the .torrent file. (or more precisely, the index into the "files" list in the meta data section of the torrent file, or 0 for single file torrents)
@zeule commented on GitHub (Jan 19, 2016):
Thank you
@naikel commented on GitHub (Jan 19, 2016):
Well I'm not sure if that explains the crash...? It happens because there's some index of a file that doesn't have a name. So when this line from file_path is called:
It crashes in fe.filename() because it calls std::string(name, name_len) with name being an invalid pointer, and I think that only happens when the index is a pad file.
@arvidn commented on GitHub (Jan 19, 2016):
filename() (if the file hasn't been renamed) is supposed to point into the metadata buffer. One potential problem is if the file_storage or torrent_info objects have moved, perhaps this pointer hasn't been updated to move with it?
@arvidn commented on GitHub (Jan 19, 2016):
I think it's an immutable buffer held by a shared_ptr<> though, so that shouldn't be a problem
@naikel commented on GitHub (Jan 19, 2016):
Ok, a more direct question:
Can you call libtorrent::file_path with an index of a pad file? Because in every single line in libtorrent where you call the method file_path, you validate if pad_file_at(index) is false first, even in storage::initialize(), so that's why I think pad files don't have a file_path, and hence the crash when the developer who implemented this tried to do it.
@arvidn commented on GitHub (Jan 19, 2016):
examples/dump_torrent.cpp calls file_path() for every file, including padfiles.
have you ruled out that the index is out-of-range? the check that's there is only enabled in debug builds.
@hdcTenBasePid commented on GitHub (Jan 19, 2016):
I've managed to do it again, but in different circumstances.
Was only seeding, and on a different machine (still 10586.63).
I believe I was looking at the peers, crash, but qBT was still seeding behind the crash report.
Apart from different sequences of dlls (different configuration) the backtrace is identical except for Line #9 from the orginal trace being missing in this one.
qBcrash20160120.1203.txt
@arvidn commented on GitHub (Jan 19, 2016):
where does the torrent_info object come from? I assume it's returned as a shared_ptr<> from the torrent_handle. is it held by a shared_ptr<> or do you take a local copy of it? (I would recommend the former).
There is one other thing I should probably mention. There's a feature in 1.1rc where torrents can be evicted from memory if they stay inactive and stopped for too long, and if they're needed again a client-supplied load function is called that's supposed to read it back in from disk. This feature only applies to torrents that don't have the "pinned" flag in their add_torrent_params (which is set by default). It also should not be enabled if the client has not supplied a load function, so I doubt that this is the case here. However, since we're dealing with a bug, perhaps there's a bug causing this eviction to trigger even though it shouldn't. Asking for the torrent_info() should always load it back in though.
@glassez commented on GitHub (Jan 20, 2016):
We should check the arguments. At least,
(s.file_index >= 0) && (s.file_index < filesCount()).@zeule commented on GitHub (Jan 20, 2016):
@glassez , do you mean a bug in
map_block()or thatpieceIndexis out of range?@glassez commented on GitHub (Jan 20, 2016):
Anything can happen... We can, at least, check it out.
The question is, can someone from the developers to reproduce this bug. If Yes, then you can simply output a debug message with these values. If not, then it is more difficult. How to make a normal user able to get this information and pass it here?
@zeule commented on GitHub (Jan 20, 2016):
Then we need a logging.
@naikel commented on GitHub (Jan 20, 2016):
What we need is a torrent file that caused this to try to reproduce it.
@Nemo-qB commented on GitHub (Jan 20, 2016):
I've tried to reproduce the crash, even with a Bitcomet created torrent file with padding files with no succes. See here: https://github.com/qbittorrent/qBittorrent/issues/4597#issuecomment-173022274
@sledgehammer999 commented on GitHub (Jan 20, 2016):
@Nemo-qB FYI when you edit your post no one is notified.
There doesn't seem to be anyone from the devs that is able to reproduce it. So I am thinking of going ahead and implement the range check and release a v3.3.3 ASAP.
@glassez commented on GitHub (Jan 20, 2016):
+1 if this is a problem of any particular file.
In any case, it doesn't always happen. And it's unclear what it depends on...
@sledgehammer999 commented on GitHub (Jan 20, 2016):
I just closed a few duplicates. 2 people seem to be able to reproduce. I asked them to post or email the offending torrent. Let's see what happens.
@zeule commented on GitHub (Jan 20, 2016):
I would also check that
pieceIndex < torrent_info::num_pieces()@glassez commented on GitHub (Jan 20, 2016):
Yes. And also
pieceIndex >= 0We can't be sure of any of the values involved here.
@ccloli commented on GitHub (Jan 20, 2016):
Here is a torrent that crashed qBittorrent when I opened an issue 4 hours ago. It's still downloading, but when I clicked the "Peers" button, qBittorrent doesn't crash and works fine now. Maybe some
particular peer information crash qBittorrent, but I'm not sure and I can't reproduce it right now. BTW, showing peers on network interface doesn't crash it.
Magnet Link: magnet:?xt=urn:btih:1097c2db007b3b05f5c9a7afa25c8319e874de3e
NOTICE: This torrent contains ADULT content, I'm sorry that it may trouble you.
It seems that the torrent is not created by BitComet, because I don't see the padding files. And when I added it to qBittorrent, it doesn't contain any tracker, but now it has some trackers (I enabled Exchange trackers with other peers on setting).
I also uploaded the torrent file here: http://tempsend.com/D8C6B01A78
@sledgehammer999 commented on GitHub (Jan 20, 2016):
An interesting attribute of your torrent is that the filenames have asian characters. So maybe the actual problem is incorrect utf8 handling by libtorrent/qbittorrent. Did anyone that had a crash also have torrents with non-english characters in the filenames?
@sledgehammer999 commented on GitHub (Jan 20, 2016):
This might be a string problem indeed. I can get it to crash with that torrent after a while. Here is some clipped output from the debugger:
@naikel commented on GitHub (Jan 20, 2016):
Is this only happening in Windows? I might never reproduce it then 😞
@glassez commented on GitHub (Jan 20, 2016):
Bug confirmed with this torrent.
But why now? I mean starting from v3.3.2.
@ccloli commented on GitHub (Jan 20, 2016):
It would probably from the new feature that has been mentioned at https://github.com/qbittorrent/qBittorrent/issues/4597#issuecomment-172794694
@glassez commented on GitHub (Jan 20, 2016):
Clear. Another is unclear. It seems that this happens when you access the file name. But these same functions are used everywhere in the program, and not only in this new feature.
@sledgehammer999 commented on GitHub (Jan 20, 2016):
Minor update: I am currently running a custom version what will print "out of bounds" piece index or files index. And now I am waiting for it to crash...
@hdcTenBasePid commented on GitHub (Jan 20, 2016):
I can see your all making progess. Didn’t even notice the new column in Peers (never maximised screen).
Finally worked out the torrents effective at the crash times. Only included the first Tracker for brevity:
PS: Luv your (the gang) work.
SeedingLeech.10Home.txt
SeedingOnly.10Pro.txt
@sledgehammer999 commented on GitHub (Jan 20, 2016):
Minor update: I am currently running a custom version what will print "out of bounds" piece index or files index. And now I am waiting for it to crash...
It just crashed. I didn't get any printout of "out of bounds" piece or file index...
@naikel commented on GitHub (Jan 20, 2016):
That's kinda bad news, because now you'd have to insert debug lines in libtorrent to see why file_storage::file_path and/or internal_file_entry::filename crash.
So far with that torrent I haven't seen a crash in Linux...
@glassez commented on GitHub (Jan 20, 2016):
And, maybe, try to insert
try-catcharound filePath() call? Incatchwe can suppres program crash and print some debug info. @sledgehammer999, It will also be useful as workaround to get rid of crashes in future versions, until we find a solution.@asturel commented on GitHub (Jan 20, 2016):
I dont rly see a pattern in torrents what cause crash (none of my torrents had special characters, but they where ~8-16GB), how can I debug the functions values? Just attach the process/start debugging in Qt Creator? Does it requires qbt to be compiled in debug mode?
@sledgehammer999 commented on GitHub (Jan 20, 2016):
It's been one day from release and a lot of crash reports are coming in. I am not confident that I'll find/fix the bug today. And tomorrow I'll be out of town for most of the day. I'll disable temporarily that feature and release v3.3.3.
@glassez if in the meantime you want to continue debugging be my guest.
I am thinking of outputting in a txt file the values of the local vars of each function. Do you guys have any better idea?
@naikel commented on GitHub (Jan 20, 2016):
I'm in no way discouraging anybody or any feature but I wonder how is it useful to know just for an instant of a second what file a peer is downloading a piece? To me it changes very fast and sometimes I can't even read what file was...
@Nemo-qB commented on GitHub (Jan 20, 2016):
+1 to naikel.
@arvidn commented on GitHub (Jan 20, 2016):
from that debugger output:
int _Count = 0x9d78558
that's the length of the string passed in to the std::string constructor. This implies that the internal_file_entry itself is invalid, for whatever reason, not just the string pointer
@arvidn commented on GitHub (Jan 20, 2016):
is this crash new with libtorrent 1.0.8?
@naikel commented on GitHub (Jan 20, 2016):
I'm trying to reproduce it in Linux with 1.0.6 and I just can't, but now I don't know if it's Linux or libtorrent 1.0.6 that doesn't have the bug...
@sledgehammer999 commented on GitHub (Jan 20, 2016):
I can't tell. Previous qbt version didn't have the offending feature implemented and it used 1.0.7.
As a matter of fact I'll do a test with 1.0.7. It is easy leaving it in the background while doing other stuff.
@glassez commented on GitHub (Jan 20, 2016):
For me this is also a completely useless thing.
No. I reproduced this with 1.0.7.
@sledgehammer999 commented on GitHub (Jan 20, 2016):
Phew. You saved me time.
@glassez commented on GitHub (Jan 20, 2016):
I seen into internal_file_entry code a little. It should limit the string length. If 0x9d78558 is greater than max Len value then it clearly bug there.
@sledgehammer999 commented on GitHub (Jan 20, 2016):
@naikel I got a crash on Debian sid (libtorrent 1.0.7) with another random torrent:
I don't think it will help but here is gdb bt
@naikel commented on GitHub (Jan 20, 2016):
Wow. This gets even weirder by the minute. In that crash in file_storage::file_path the string m_paths[fe.path_index] is invalid.
That can be for several reasons but I think it's because the internal_file_entry structure (from m_files[index]) is totally invalid, and that's from m_files[1], because index=1.
@naikel commented on GitHub (Jan 20, 2016):
More info:
1.- That torrent has a single file and that should be m_files[0], why is it trying to access m_files[1]?
2.- Was that a libtorrent with --enable-debug? why the TORRENT_ASSERT_PRECOND didn't say anthing about index >= m_files.size()?
3.- Looks like map_block for an specific piece gives a file_slice with file_index >= num_files. In the particular case of this torrent, s.file_index has always to be zero. If it's one, it will crash.
@sledgehammer999 commented on GitHub (Jan 20, 2016):
No. I used the one from the debian repos. I only compiled qbt with debug.
Maybe an off-by-one bug in libtorrent?
(I wonder why my previous prints didn't detect the out of bounds index).
@naikel commented on GitHub (Jan 20, 2016):
The thing is, if that's the problem, a check before calling filePath that file_index < num_files is enough, even if map_block in libtorrent has a bug.
@arvidn commented on GitHub (Jan 20, 2016):
If you're debugging optimized builds, I would take what the debugger is saying with a grain of salt. and I would recommend testing debug builds of libtorrent as well libstdc++, by defining _GLIBCXX_DEBUG or _GLIBCXX_ASSERT (the latter preserves ABI).
It looks like the internal_file_entry is invalid for some reason. It could be an index out-of-range or the underlying vector may have been reallocated or the underlying object that holds the file_storage object may have been destroyed.
I could add more debug tracking and asserts around this function, but unless you build in debug mode, it won't do much good
@glassez commented on GitHub (Jan 21, 2016):
Damn, I just downloaded affected torrent from beginning to end without crash!
I don't like this bug...
@Nemo-qB commented on GitHub (Jan 21, 2016):
And qBittorrent is stable again with v3.3.3 :). Thanks for the new build.
@zywo commented on GitHub (Jan 21, 2016):
I reported the issue 17 days ago: https://github.com/qbittorrent/qBittorrent/issues/4501
Ubuntu 15.10
v3.4.0alpha
qt 4.8.6
libtorrent 1.0.6.0
boost 1.58.0
@glassez commented on GitHub (Jan 21, 2016):
@zywo how many files in affected torrent? (when program crashed as you reported in #4501)
@glassez commented on GitHub (Jan 21, 2016):
@zywo can you provide this torrent for testing? (unless it's some private torrent of course)
@zywo commented on GitHub (Jan 21, 2016):
@glassez I don't remember how many files.
I just reproduced the crash with this torrent: af420a4945e5d8fec790202541903eae2e924a0f
@glassez commented on GitHub (Jan 21, 2016):
@zywo, Please describe more the circumstances of this event. What tabs were opened. The main window has been shown or minimized, etc. How much (approx.) time has passed since the launch of torrent.
@zywo commented on GitHub (Jan 21, 2016):
@hdcTenBasePid commented on GitHub (Jan 21, 2016):
Just to muddy the waters,
Previous to this ,but in the same session I experienced #4611, which I let run for about an hour, then cancelled, and started again, magnets. (Second time good.)
Now to move on for the moment will check out 3.3.3. Happy hunting ...
@glassez commented on GitHub (Jan 21, 2016):
Catched!
I downloaded this torrent a few times and got the crash in all cases except one. I slightly modified the code to suppress the crash and print some information into the log:
And here's what I got every time this has happened (torrent has 1 file):
It turns out that this is a bug in the library, with the result that `torrent_info::map_block()`` returns extra file for the last part (for some reason, not constantly).
@arvidn?
@arvidn commented on GitHub (Jan 21, 2016):
there's a subtle issue when using map_file() (and I imagine the same is true for map_block()) for the last byte of the torrent (or more likely, the one-past-the-end byte). in the dump_torrent.cpp example there's some careful arithmetic to make sure that the returned ranges is within the torrent. specifically, take a look at this line:
https://github.com/arvidn/libtorrent/blob/master/examples/dump_torrent.cpp#L197
to map a file to a first-piece and a last-piece, map_file() is invoked twice, once for the first byte of the file, and once for the last byte of the file.
What's the context you use map_block() in? could you provide a link to the source?
@zeule commented on GitHub (Jan 21, 2016):
We call it with arguments (pieceIndex, 0, torrent_info::piece_length()). PieceIndex we obtain from peer_info::downloading_piece_index.
Since the code was reverted here is the original pull request
The code is in src/base/bittorrent/torrentinfo.cpp
@arvidn commented on GitHub (Jan 21, 2016):
I think the problem is that you pass in the generic piece_length() as the size of the block. If the last piece is shorter than that, you'll be mapping space past the end of the torrent. In debug builds this should yield an assert. If you instead pass in nativeInfo()->piece_size(pieceIndex) I imagine it would work
@arvidn commented on GitHub (Jan 21, 2016):
given that this is such an easy mistake to make, I should probably add a run-time check and just truncate the response
@naikel commented on GitHub (Jan 21, 2016):
Does this mean different pieces can have different sizes? I didn't know Bittorrent protocol allowed that...
@arvidn commented on GitHub (Jan 21, 2016):
every piece except for the last one have the same size.. but it's not required the size of the torrent be divisible by that piece size, so the last piece is most likely shorter
@zeule commented on GitHub (Jan 21, 2016):
All this looks very clear now. Thanks a lot! And indeed, either a note in the documentation or truncation of the values which are out of range would be nice to have.
@arvidn commented on GitHub (Jan 21, 2016):
I would highly recommend using a debug build of libtorrent during development. if all the invariant checks in there are too slow, they can be disabled. I would expect an assert to fire consistently on any seeding torrent that invokes this (in debug mode)
@sledgehammer999 commented on GitHub (Mar 17, 2016):
Fixed with #4867