fileio: Fix hang on error [DEVINFRA-601] #396
Conversation
Fixes a hang in fileio that would occur if any command had an error during processing, causing the sbp reading thread to wait forever due to never receiving its exit condition. This moves all the potentially error-producing functions into closures, so that the errors can be handled together.
The MSG_FILEIO_READ_DIR_RESP isn't enable to be output by default on Piksis, so it would be useful to provide a hint to users as to why the command may be timing out.
| timestamp = packet.timestamp, | ||
| level = packet.level, | ||
| msg = packet.msg, | ||
| spaces = spaces, |
There was a problem hiding this comment.
This was causing issues for me (I guess I need to update to 1.58.0?) but figured the rest of the arguments are specified, so this one should probably be too, so I fixed it for consistency. 🤷♂️
| print!("\rWriting {:.2}%...", progress); | ||
| })?; | ||
| println!("\nFile written successfully ({} bytes).", bytes_written); | ||
| let res = (|| { |
There was a problem hiding this comment.
This is kind of awkward, but is nice for convenience. I guess we could change each of these closures to be actual functions, or maybe we could use try_block for these? Happy to go with whatever people think is most readable.
Prints to stdout instead of stderror and prints each entry without the list formatting
In #395 I switched from crossbeam threads to std::threads, which I think will also solve the issue? I'm assuming that the hanging was at the end of the closure passed to Would wanna double check that this indeed fixes this issue, but if it does it's IMO a cleaner solution. It also avoids having to have the dedicated channel to kill the reader thread which is nice |
Ah, crap, sorry didn't notice that you had opened that! Will close this in preference to that PR then. |
4 participants
Fixes a hang in fileio that would occur if any command had an error
during processing, causing the sbp reading thread to wait forever due to
never receiving its exit condition.
This moves all the potentially error-producing functions into closures,
so that the errors can be handled together.
Also:
listmay not be working(
MSG_FILEIO_READ_DIR_RESPdoesn't look like it's enabled bydefault)
instead of to stderr) and prints each entry without the vec formatting.
I don't know for sure, but I suspect Stefan was seeing a hang as his PIksi may not have had
MSG_FILEIO_READ_DIR_RESPenabled. With this message enabled, thelistcommand did seem to work fine for me. This change will at least ensure the error is shown, if there is one.