Comment 1 by Thomas Keller, Jan 17, 2011
I obviously forgot the "remote_stdio in remote_stdio" pair which should also be prevented. I also tested out remote in remote_stdio and this worked just well. remote in stdio worked as well, but leaves some out-of-band gunk that breaks stdio. I'll try to fix that and afterwards enable remote in stdio and remote_stdio (and of course I'll write test for that so it does not break again).
Status:
Accepted
Owner: tommyd
Owner: tommyd
Comment 2 by Timothy Brownawell, Jan 18, 2011
Calling remote/push/pull/sync via remote or remote_stdio will block the server's event loop (well, everything really blocks the event loop for its duration. But network commands tend to be slower.). Calling *stdio inside *stdio doesn't make much sense because you have to provide all the input up front (or maybe it's still useful just as a batching mechanism). Calling stdio inside either stdio or remote_stdio will screw with sanity::set_out_of_band_handler, because that isn't reentrant (it would need to be replace/restore instead of set/reset). Calling remote_stdio doesn't do this, because the handler is on the target server. I think all we really *need* to prevent is stdio inside of either stdio or remote_stdio, but the get_remote_automate_permitted documentation should note that network commands and stdio might stall the event loop longer than other things.
Sign in to reply to this comment.
Reported by Thomas Keller, Jan 12, 2011
Steps to reproduce the problem: ------------------------------- Write a small hook / user command which calls mtn_automate("remote", "--remote-stdio-host= mtn://code.monotone.ca/monotone", "branches") Expected result: ---------------- See the list of branches from the remote monotone server. Actual results: --------------- automate.cc:2448: detected network error, 'E(acmd->can_run_from_stdio())' violated Output of `mtn version --full`: ------------------------------- 0.99.1 and probably earlier. This was likely introduced when the stdio setup code was refactored. A quick fix might be to add a setter for stdio_ok in cmd_automate.cc (set_can_run_from_stdio) and call this setter later in the same file in LUAEXT(mtn_automate) conditionally when the command "remote" should be executed. Maybe it would also be a good idea to rethink the stdio_ok flag, i.e. why shouldn't it be allowed to call "remote" also via stdio? Things we'd like to block because of otherwise mixed up communication streams are * calling stdio in remote_stdio * calling stdio in stdio * calling remote_stdio in stdio * calling remote inside remote_stdio (never tried that, it could _theoretically_ work, but it also might break horribly) So the first four items are blocked nicely with the existing stdio_ok, maybe we should just find out whether it would hurt us a lot to switch CMD_AUTOMATE_NO_STDIO(remote) to CMD_AUTOMATE(remote).