search

arvidn / libtorrent

an efficient feature complete C++ bittorrent implementation
http://libtorrent.org
Other
5.16k stars 994 forks source link

assert in disk_io_thread::async_read #3601

Closed MassaRoddel closed 5 years ago

MassaRoddel commented 5 years ago

libtorrent version (or branch): master rev 144647

platform/architecture: windows

compiler and compiler version: vs2015

I get assert TORRENT_ASSERT(r.start % default_block_size == 0). Is it not allowed to request blocks smaller than the 16k which could have an offset within 16k?

arvidn commented 5 years ago

Is this with a torrent with a piece size smaller than 16k?

MassaRoddel commented 5 years ago

No, just that the clients (Xunlei so far) request small amounts of data (less than 16k, could be a few hundred bytes) with offsets not aligned to 16k.

arvidn commented 5 years ago

interesting. that should clearly be supported. I should add an option to do that in connection_tester, and ideally have a unit test for it as well

arvidn commented 5 years ago

the reason for this assert is that the store buffer (where pending disk writes are queued) only supports lookups by block number.

I could make async_read a bit more sophisticated, to still look up the right block, and use an offset into it

arvidn commented 5 years ago

I'm pretty sure this will fix it: https://github.com/arvidn/libtorrent/pull/3680

arvidn commented 5 years ago

please let me know if you have a chance to test it.

MassaRoddel commented 5 years ago

Should this build with master branch?

arvidn commented 5 years ago

Yes

MassaRoddel commented 5 years ago

I get several errors like disk_buffer_holder has no member get(). Is that now data()?

arvidn commented 5 years ago

yes, but all those places were changed. I don't know how you would end up with such inconsistency.

you can either apply this patch to your master (and I don't think the patch mentions .get()) or you update your repo to the pull requests commit, and you get all previous changes in master along with it.

MassaRoddel commented 5 years ago

I use the svn bridge https://github.com/arvidn/libtorrent/trunk. Does this no longer refer to master? i.e. I still have block_cache.cpp

arvidn commented 5 years ago

I'm not really familiar with any svn bridge. is that something run by github? That link seems dead for instance. Does the svn bridge not give you access to arbitrary git branches? if it does, you could just pull the PR branch

MassaRoddel commented 5 years ago

Github runs the bridge so that you simple can connect with a svn client. The link works with svn client for me not with a browser.

MassaRoddel commented 5 years ago

I have run some tests with your fix and I got no asserts so far. (used https://github.com/arvidn/libtorrent.git/branches/master)

Github links svn trunk to the default git branch. Currently https://github.com/arvidn/libtorrent/trunk gets you the RC_1_2 branch. That is why I got some problems.

arvidn commented 5 years ago

I see. Thanks!