Sender hangs when the file size is a multiple of 4096

[kanna5]

To reproduce:

dd if=/dev/zero of=test.bin bs=4096 count=123
wormhole-rs send test.bin

The receiver will exit once all the contents of the file have been received, but the sender will hang until ctrl+c is pressed twice.

[aeshirey]

Newbie here interested in contributing.

It looks like the cause of this is in transfer::v1::send_records where the async read is performed. In the case of an input of size 4096, there's nothing left to read, so the sender somehow just spins. ¯\_(ツ)_/¯

Fortunately, this function receives file_size, so it looks like this should be a straightforward fix:

if n < 4096 {
    break;
} else if sent_size == file_size {
    // multiple of 4096
    break;
}

I already verified that @kanna5's dd input hangs the sender and that this additional check fixes it. Happy to submit a PR. I'm curious why the current check doesn't simply do this, though? That is, wouldn't it be better to change the current code to simply:

if sent_size == file_size {
    break;
}

[piegamesde]

Thanks for having a look. I'm not sure about checking sent_size == file_size, since this may go bad if the file is modified concurrently (time of check vs time of use). We could do if n < 4096 || sent_size >= file_size, but this would still hang if the file was concurrently truncated to a multiple of 4096 bytes. This would be an acceptable compromise for me, but there really should be a better solution to this.

---

Edit: after reading the documentation, what we currently do should be mostly correct. The last read call should return 0, indicating an EOF. This is also what read_to_end relies on internally. I therefore see two possible causes of the issue here: 1. It could be from the fact that we are async and things are subtly different there. 2. The last read call indeed returns zero and we then call transit.send_record with an empty slice, which may turn out weird. Adding a if n == 0 { break } directly after the read might resolve this then.

[aeshirey]

Ah, you're right. In my rudimentary testing, it didn't appear to be the case:

eprintln!("Reading...");
let n = file.read(&mut plaintext[..]).await?;
eprintln!("Read {n} bytes");

"Read 0 bytes" never showed up, maybe the progress bar hampering stdout? But assert!(n > 0) crashes.