boostorg / process

Boost Process
https://www.boost.org/libs/process
117 stars 116 forks source link

Deadlock when using async input and pipes. #64

Open davisking opened 5 years ago

davisking commented 5 years ago

Should this work, or is there a bug in what I'm doing? With the latest code on github, the following program hangs:

    std::string data = "some data to send to cat";
    std::future<std::string> result;
    boost::asio::io_service ios;
    bp::pipe p;
    bp::spawn("cat", bp::std_in < bp::buffer(data), bp::std_out > p, ios);
    bp::spawn("cat", bp::std_in < p, bp::std_out > result, ios);

    // run blocks forever.
    ios.run();
    std::cout << result.get() << std::endl;

This seems like something that ought to work. Also, these two other programs that are only a little different work as expected, making it seem like there might be a bug in the interaction between multiple processes participating in a pipeline and the use of async input:

    // use echo to make data instead of bp::buffer() 
    std::future<std::string> result;
    boost::asio::io_service ios;
    bp::pipe p1,p2;
    bp::spawn("echo -n some data to send to cat", bp::std_out > p1);
    bp::spawn("cat", bp::std_in < p1, bp::std_out > p2);
    bp::spawn("cat", bp::std_in < p2, bp::std_out > result, ios);

    ios.run();
    std::cout << result.get() << std::endl;
    // When pipes aren't used everything also works as expected.
    std::string data = "some data to send to cat";
    std::future<std::string> result;
    boost::asio::io_service ios;
    bp::spawn("cat", bp::std_in < bp::buffer(data), bp::std_out > result, ios);

    ios.run();
    std::cout << result.get() << std::endl;
klemens-morgenstern commented 5 years ago

I think it's a bug in the async stdin version, because (I fear) that doesn't close the pipe when done. Thus cat stays active since the pipe is open.

Can you test using a different input method? i won't be able to look into it before next week.

davisking commented 5 years ago

I've found a number of other seemingly related deadlocks. Like maybe https://github.com/boostorg/process/issues/65. As well as this:

    std::string data = "some data to send to cat";
    std::future<std::string> result;
    boost::asio::io_service ios;
    boost::process::pipe p;
    opstream out;
    spawn("cat", std_in<out, std_out>p);
    spawn("cat", std_in<p, std_out>result, ios);

    std::thread t([&]() { out << data << std::flush; out.pipe().close(); });
    // run blocks forever.
    ios.run();
    t.join();

However, this slightly different version works as expected:

    std::string data = "some data to send to cat";
    std::future<std::string> result;
    boost::asio::io_service ios;
    boost::process::pipe p;
    opstream out;
    spawn("cat", std_in<out, std_out>result, ios);

    std::thread t([&]() { out << data << std::flush; out.pipe().close(); });
    ios.run();
    t.join();

Maybe they are all caused by the same underlying issue. I am not sure.

klemens-morgenstern commented 5 years ago

I assume you're on linux, right? Can you try out what happens if you change spawn to async_system and put some dummy handler in?

I.e. async_system(ios, [](const boost::system::error_code & ec, int ret){}, ("cat", std_in<out, std_out>p);

davisking commented 5 years ago

Yeah, using Ubuntu 16.04.

I tried replacing spawn with async_system and it still hangs just like before.

klemens-morgenstern commented 5 years ago

Weird. Alright, I'll look into it. It might actually be the same bug for all your issues.

davisking commented 5 years ago

Sweet, thanks :)

Yeah, that's my suspicion as well.

klemens-morgenstern commented 5 years ago

Do you use boost 1.68 or the current HEAD of the git repo?

davisking commented 5 years ago

I'm using Boost 1.66 along with the code from https://github.com/boostorg/process as it was on Jan 1st.

klemens-morgenstern commented 5 years ago

Ok, that should, but could you use the currend develop-HEAD just to be sure?

davisking commented 5 years ago

Just grabbed the develop-HEAD of https://github.com/boostorg/process and got the same results, same things work and same things still hang.

LuisAyuso commented 5 years ago

I am experimenting sort of the same issue. Although my code only binds stderr and stdout output streams, whenever I execute the context run function it will hang in there even when the child process is gone.

something like:

    child = boost::process::child(path, args, group, bp::std_out > stdout_stream, bp::std_err > stderr_stream, ios);
[..]
    ios.run();
    child.wait();

When I submit a kill signal to the child process, the parent process is still waiting for IO. child.wait() is never executed.

Is this the same issue? If so, could we rename this thread to remove the input keyword?

klemens-morgenstern commented 5 years ago

@LuisAyuso I would need more about your use-case - do the streams refer to an async_pipe?

LuisAyuso commented 5 years ago

my stream objects are a custom class, they wrap an async_pipe, hold a buffer to accumulate output, and re-register the callback after every read. I noticed that the read callback is never invoked after child process dead, not even to notify an error.

klemens-morgenstern commented 5 years ago

Weird, which system are you on? Normally you should get an EOF.

What do you call to start the read operation?

LuisAyuso commented 5 years ago

Sorry for fragmented information, let's see if we manage to have a proper report ;) I am using Ubuntu 18.04, with gcc 7.4.0 The Boost version is 1.69

I have been cleaning the code so I can show something here:

    ba::io_context ios;

    std::vector<char> stdout_buffer(1024);
    auto              out_buffer{ba::buffer(stdout_buffer)};
    bp::async_pipe    std_out{ios};
    std::function<void(const boost::system::error_code &ec, size_t s)> stdout_callback;
    stdout_callback = [&](const boost::system::error_code &ec, size_t s) {
        if (ec)
            return;
        auto v = std::string_view(&stdout_buffer.front(), s);
        fmt::print("stdout: {}", v);
        std_out.async_read_some(out_buffer, stdout_callback);
    };

    std::vector<char> stderr_buffer(1024);
    auto              err_buffer{ba::buffer(stderr_buffer)};
    bp::async_pipe    std_err{ios};
    std::function<void(const boost::system::error_code &ec, size_t s)> stderr_callback;
    stderr_callback = [&](const boost::system::error_code &ec, size_t s) {
        if (ec)
            return;
        auto v = std::string_view(&stdout_buffer.front(), s);
        fmt::print("stderr: {}", v);
        std_err.async_read_some(err_buffer, stderr_callback);
    };

    std_out.async_read_some(out_buffer, stdout_callback);
    std_err.async_read_some(err_buffer, stderr_callback);

    child =
          bp::child(path, args, group, bp::std_out > std_out, bp::std_err > std_err, ios);

    ios.run();
    child.wait();

The behavior of this code is the same as the one reported originally.

klemens-morgenstern commented 5 years ago

This is weird indeed. Can you add bp::on_exit to check if this gets called?

Also, where does the group come from?

LuisAyuso commented 5 years ago

Group is currently default initialized. (bp::group)

on_exit works, I can use it now to shut down the io context (ios.stop()) and let the program proceed to child.wait() I can use now this as a workaround, it is way better than my original one (which was to put io.run in a separate thread)

klemens-morgenstern commented 5 years ago

The better workaround would be to close the pipes from the on-exit handler.

Can you try the current master branch and see if it fixes the issue?

xd009642 commented 4 years ago

So I encountered this issue today, but I had also encountered the same effect in some rust async code in tokio. So because of the possibility they could be the same issue here is my associated issue and more importantly the PR that fixed it https://github.com/tokio-rs/tokio/issues/2255 https://github.com/tokio-rs/tokio/pull/2218

Spixmaster commented 1 month ago

@klemens-morgenstern, I still have the same issue even with v2.

boost::asio::io_context io_context;
boost::asio::writable_pipe std_in(io_context);
boost::system::error_code error_code_stdin;
boost::asio::write(std_in, boost::asio::buffer("asdf"), error_code_stdin);

const std::int32_t exit_code = boost::process::v2::execute(boost::process::v2::process(
  io_context, boost::process::v2::environment::find_executable("cat"), {},
  boost::process::v2::process_stdio {std_in, {}, {}}));
//First point that is not reached.
klemens-morgenstern commented 1 month ago

You're using execute which is blocking, which waits for subprocess to exit. And cat has an open pipe as stdio, which means it won't exit.

Spixmaster commented 1 month ago

Can you give a working example, please?

What do you mean with "open pipe"? Running echo "a" | cat prints a. cat prints out what it received via stdin. Is not it possible to use cat with boost::process::v2::process and stdin?

klemens-morgenstern commented 1 month ago

You provided an example:

  1. execute waits for cat to exit (unlike v2::process)
  2. cat waits for the input pipe to be closed or a signal
boost::asio::io_context io_context;boost::asio::writable_pipe std_in(io_context);
boost::system::error_code error_code_stdin;

boost::process::v2::process cat(  io_context, boost::process::v2::environment::find_executable("cat"), {},  boost::process::v2::process_stdio {std_in, {}, {}});

boost::asio::write(std_in, boost::asio::buffer("asdf"), error_code_stdin);
std_in.close();

const auto int exit_code = cat.wait();