Open ArniDagur opened 5 years ago
Can you try again with RUST_BACKTRACE=1
set? The backtrace gives additional information where the slice was attempted and makes it easier to figure out where to place additional length checks.
I ran it twice with backtrace enabled. On the second run, the client hasn't crashed yet.
torrent@arni-pc ~ $ RUST_BACKTRACE=1 synapse
09/05/19 22:37:14 [synapse::init:24] I: Initializing
09/05/19 22:37:14 [synapse::config:173] I: Loaded config file
09/05/19 22:37:14 [synapse:112] I: Initialized, starting!
09/05/19 22:37:14 [synapse::rpc::processor:92] I: user data could not be read from disk, creating a fresh version: No such file or directory (os error 2)
09/05/19 22:37:14 [synapse::rpc:603] I: RPC SSL parameters not specified, using insecure connections!
09/05/19 22:37:14 [synapse::tracker::dht::rt:102] I: Loaded 23 dht buckets, 184 nodes
09/05/19 22:37:14 [synapse::tracker::dht:57] I: Attempting DHT bootstrap!
thread 'disk' panicked at 'index 207566898 out of range for slice of length 206518322', src/libcore/slice/mod.rs:2555:5
stack backtrace:
0: backtrace::backtrace::libunwind::trace
at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/libunwind.rs:88
1: backtrace::backtrace::trace_unsynchronized
at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/mod.rs:66
2: std::sys_common::backtrace::_print
at src/libstd/sys_common/backtrace.rs:47
3: std::sys_common::backtrace::print
at src/libstd/sys_common/backtrace.rs:36
4: std::panicking::default_hook::{{closure}}
at src/libstd/panicking.rs:200
5: std::panicking::default_hook
at src/libstd/panicking.rs:214
6: std::panicking::rust_panic_with_hook
at src/libstd/panicking.rs:477
7: std::panicking::continue_panic_fmt
at src/libstd/panicking.rs:384
8: rust_begin_unwind
at src/libstd/panicking.rs:311
9: core::panicking::panic_fmt
at src/libcore/panicking.rs:85
10: core::slice::slice_index_len_fail
at src/libcore/slice/mod.rs:2555
11: synapse::disk::cache::FileCache::read_file_range
at src/disk/cache.rs:0
12: synapse::disk::job::Request::execute
at src/disk/job.rs:481
13: synapse::disk::Disk::handle_active
at src/disk/mod.rs:101
14: synapse::disk::Disk::run
at src/disk/mod.rs:74
15: synapse::disk::start::{{closure}}
at src/disk/mod.rs:201
16: synapse::handle::Handle<I,O>::run::{{closure}}
at src/handle.rs:39
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
09/05/19 22:38:09 [synapse::control:145] E: crashed thread detected, terminating!
09/05/19 22:38:09 [synapse::init:41] E: Unclean shutdown detected, terminating
torrent@arni-pc ~ $ RUST_BACKTRACE=full synapse
09/05/19 22:38:22 [synapse::init:24] I: Initializing
09/05/19 22:38:22 [synapse::config:173] I: Loaded config file
09/05/19 22:38:23 [synapse:112] I: Initialized, starting!
09/05/19 22:38:23 [synapse::rpc::processor:92] I: user data could not be read from disk, creating a fresh version: No such file or directory (os error 2)
09/05/19 22:38:23 [synapse::rpc:603] I: RPC SSL parameters not specified, using insecure connections!
09/05/19 22:38:23 [synapse::tracker::dht::rt:102] I: Loaded 23 dht buckets, 184 nodes
09/05/19 22:38:23 [synapse::tracker::dht:57] I: Attempting DHT bootstrap!
09/05/19 22:39:26 [synapse::torrent:878] I: Torrent E057A8A80EE5656AAEC05333CBCFED62FAC8C7B7 completed!
09/05/19 22:39:53 [synapse::torrent:878] I: Torrent 132E4AB0B1E7D4D27B2C0B30F4BEE1868460ADF2 completed!
Thanks, there's a missing check to ensure that FileCache::read_file_range
isn't called with offset + buf.len()
larger than the file. You can work around this until it is patched by building without the mmap feature.
I'll look into this, but generally I'd avoid the mmap feature it possible, my conclusion on it is that the theoretical gains aren't worth the complexity. It will probably be removed in a future release.
I wonder how async std's performance of BufReader is in a case like this
As of right now async std currently uses a threadpool to handle all its blocking file operations. I've looked into this sort of approach but I'm fairly certain it won't give very good performance and just add more complexity. I think to achieve serious performance something like io_uring would need to be used, but based on my benchmarks this is somewhat unnecessary, synapse is rarely bottlenecked by disk read/write performance as far as I know.
@ArniDagur do you know if your torrents were complete when you tried doing hash validation on them?
My best guess as to what happened is that because in validation length isn't passed into the ensure_exists
method of the file cache, the existing file was mmap'd but wasn't large enough so the slice access went out of bounds.
Regardless I'll probably just remove the mmap code soon which I think should take care of this. In the meanwhile just don't use it.
These were all completed torrents.
Here is the full activity log:
The panic occured during the initial hash-verification of my torrents. This was my torrent list one or two minutes before, in case it gives any clues (don't judge me):