Race condition between idle and iopub messages in `evcxr_jupyter`? about evcxr HOT 4 OPEN

Hi @thomasjm, thanks for the report. Yeah, that sounds about right. The iopub is handled by a separate thread, but when execution finishes, it acquires a lock on the iopub socket and sends the idle message (function handle_shell_message in core.rs).

Output from stdout and stderr are handled in start_output_pass_through_thread. It needs to be handled somewhat separately from execution because the user's code can continue to print stuff even it finishes execution - i.e. if it spawns a thread. Although this currently only works for stderr due to limitations in the way evcxr handles stdout (which should probably be fixed).

std::thread::spawn(|| {
    for i in 0..10 {
        eprintln!("{i}");
        std::thread::sleep(std::time::Duration::from_millis(500));
    }
})

One possible fix would be to change the channel that evcxr uses to send output to the IO thread to include events that mark the start and end of execution. The IO thread could then lock the iopub socket for the duration of execution. This would prevent handle_shell_message from sending its idle message on iopub until after the IO thread had sent the last bit of output from during execution.

from evcxr.

Thanks for explaining! FWIW, here's what the spec has to say about iopub from background threads:

Asynchronous output (e.g. from background threads) may be produced after the kernel has sent the idle status message that signals the completion of the request. The handling of these out-of-order output messages is currently undefined in this specification, but the Jupyter Notebook continues to handle IOPub messages associated with a given request after the idle message has arrived, as long as the output area corresponding to that request is still active.

It seems that there's nothing to be done about background threads, but solving this for the main thread like you describe sounds great to me. Is there any way I can help?

I wouldn't say it's an urgent problem, since this mostly affects Papermill-style workflows where you run code and then immediately shut the kernel down. I added a 100ms sleep to the end of my test and that pretty much solved the flakiness by giving the kernel plenty of time to send the stdout. But this would be nice to fix so that such workflows can be more reliable.

from evcxr.

I've pushed a potential fix if you'd like to have a look, try it out etc. #354

It only fixes the issue for stdout. I don't think there's anything that can be done about stderr, since control messages (like execution complete) are only sent on stdout. Anyway, hopefully stdout is sufficient.

from evcxr.

Awesome, thanks for the quick fix! That seems to work, I got 100/100 successful test runs.

I don't think there's anything that can be done about stderr, since control messages (like execution complete) are only sent on stdout.

I don't follow here. AFAIK the spec treats stdout and stderr on equal footing--both are sent as iopub "stream" messages. Are we talking about something that's particular to the implementation of evcxr_jupyter?

from evcxr.