DaanDeMeyer / reproc

A cross-platform (C99/C++11) process library
MIT License
552 stars 65 forks source link

[Question] Use a programm in background that have an infinite loop #20

Closed Milerius closed 4 years ago

Milerius commented 5 years ago

Hello i have the following code:

#pragma once

#include <future>
#include <string>
#include <thread>
#include <reproc++/reproc.hpp>
#include <config/config.hpp>

namespace antara::mmbot
{

    class thread_safe_string_sink
    {
    public:
        thread_safe_string_sink(std::string &out, std::mutex &mutex)
                : out_(out), mutex_(mutex)
        {}

        bool operator()(const uint8_t *buffer, unsigned int size)
        {
            std::lock_guard<std::mutex> lock(mutex_);
            out_.append(reinterpret_cast<const char *>(buffer), size);
            return true;
        }

    private:
        std::string &out_;
        std::mutex &mutex_;
    };

    class mm2_client
    {
    public:
        explicit mm2_client(const antara::mmbot::config &cfg) : mmbot_cfg_(cfg)
        {
            VLOG_SCOPE_F(loguru::Verbosity_INFO, pretty_function);
            using namespace std::literals;
            std::array<std::string, 1> args = {std::filesystem::current_path() / "assets/mm2"};
            auto path = (std::filesystem::current_path() / "assets/").string();
            auto ec = background_.start(args, std::addressof(path));
            if (ec) {
                VLOG_SCOPE_F(loguru::Verbosity_ERROR, "error: %s", ec.message().c_str());
            }
            std::string output;
            std::mutex mutex;

            auto drain_async = std::async(std::launch::async, [this, &output,
                    &mutex]() {
                thread_safe_string_sink sink(output, mutex);
                return this->background_.drain(reproc::stream::out, sink);
            });

            drain_async.wait_for(2s);
            {
                std::lock_guard<std::mutex> lock(mutex);
                VLOG_SCOPE_F(loguru::Verbosity_INFO, "%s", output.c_str());
            }
        }

        ~mm2_client()
        {
            VLOG_SCOPE_F(loguru::Verbosity_INFO, pretty_function);
            auto ec = background_.stop(reproc::cleanup::kill, reproc::infinite, reproc::cleanup::terminate, reproc::milliseconds(0));
            if (ec) {
                VLOG_SCOPE_F(loguru::Verbosity_ERROR, "error: %s", ec.message().c_str());
            }
        }

    private:
        [[maybe_unused]] const antara::mmbot::config &mmbot_cfg_;
        reproc::process background_{reproc::cleanup::kill, reproc::infinite, reproc::cleanup::terminate, reproc::milliseconds(0)};
    };
}

My goal is simply to launch the program in background and continue the execution of my program that will use this program in background (mm2)

But when i write the unit tests:

TEST_CASE ("test launch mm2 in background")
    {
        auto cfg = load_mmbot_config(std::filesystem::current_path() / "assets", "mmbot_config.json");
        antara::mmbot::mm2_client mm2(cfg);

        nlohmann::json json_data = {{"method", "version"}, {"userpass", cfg.mm2_rpc_password}};
        auto resp = RestClient::post(antara::mmbot::mm2_endpoint, "application/json", json_data.dump());
        CHECK_EQ(200, resp.code);
        DVLOG_F(loguru::Verbosity_INFO, "body: %s", resp.body.c_str());
    }

The program looks like it is starting correctly but does not continue its execution and does not call the destructor, should it be launched in a separate thread? I did not really understand.

The idea of ​​the constructor, it is to launch the program in background, wait one or two seconds, display the output to check that the program is well started, and continue the execution of my main program.

When i comment the async logging part, it's seem's to work (unit tests is passing) and i have any output of version printed.

Roman Sztergbaum Komodo Software Developer and Blockchain Architect

DaanDeMeyer commented 5 years ago

I think this is causing the issue. A future returned by std::async has a blocking destructor. Since the program you launch never exits, the main program is blocked in the destructor of drain_async.

DaanDeMeyer commented 5 years ago

The child process might also run into issues if you never drain the output depending on how much output it produces. If reproc's stdout pipe reaches max capacity the child process might run into trouble as writes to stdout will start to fail because the pipe is full. However, if your program does not produce any more output after starting this should not be an issue.

Milerius commented 5 years ago

The child process will write a lot of opuput there is a way to automatically flush the output every x seconds ?

Milerius commented 5 years ago

or ignore output (i dont need it actually)

DaanDeMeyer commented 5 years ago

Flushing is not supported as that would require reproc to start its own thread internally which would be out of scope.

You could try closing REPROC_STREAM_OUT using reproc_close but that also depends on the child process being able to deal with a closed output pipe since writes to the output pipe will fail.

Flushing should not be terribly hard, you can make a sink that just discards output instead of saving it and then save the drain_async in mm2_client and it will continously read and discard any output from the child process. Although in that case I would recommend just using a std::thread instead of a future.

DaanDeMeyer commented 5 years ago

I added a discard sink that discards all output read from a stream which will be available in the next version of reproc that should release this month.

Milerius commented 5 years ago

i use master release atm

DaanDeMeyer commented 5 years ago

reproc::process background_{reproc::cleanup::kill, reproc::infinite, reproc::cleanup::terminate, reproc::milliseconds(0)};

Specifying more cleanup actions after any cleanup action with duration reproc::infinite has no point as reproc will wait forever for the process to stop after executing the action associated with the reproc::infinite duration.

Furthermore, reproc::kill is the most extreme cleanup action. If that one doesn't work, there's no use in following up with reproc::terminate.

This:

reproc::process background_{reproc::cleanup::kill, reproc::infinite};

would do exactly the same as what you have now although I would recommend:

reproc::process background_{reproc::cleanup::terminate, /* any reasonable waiting time */, reproc::cleanup::kill, reproc::infinite};

When using reproc::cleanup::terminate the child process has a chance to cleanup in a signal handler which is not the case when using reproc::cleanup::kill.

Of course if the child process has no important cleanup to perform you can simply execute reproc::cleanup::kill.

Milerius commented 5 years ago

Thank's for the explanation

Milerius commented 5 years ago

Can i use the new sink that you just write ? and how i should use it ?

DaanDeMeyer commented 5 years ago

You can start a thread instead of a future to flush the stream:

#include <reproc/sink.hpp>

...

std::thread([&background_]() {
  background_.drain(reproc::stream::out, reproc::sink::discard());
});

Then save that thread in mm2_client so it doesn't go out of scope and so you can clean it up in the destructor.

To check if the process started correctly, simply do:

auto error = background_.wait(2s);

if (error != reproc::error::wait_timeout) {
  // The process is not running anymore, something likely went wrong.
}
Milerius commented 5 years ago

Thank's a lot.

Milerius commented 5 years ago

Just a last question, one windows do you append '.exe' for executable name ?

DaanDeMeyer commented 5 years ago

In the cmake-help example I don't append '.exe' so I think Windows can handle executable names without the '.exe' suffix.

Milerius commented 5 years ago

ok great !

Milerius commented 5 years ago

Thank's a lot !

DaanDeMeyer commented 5 years ago

According to the CreateProcess documentation, .exe is only appended if the filename is not a path so that might be something to keep in mind.

I could look into adding it in reproc if it turns out to be a problem.

Milerius commented 5 years ago

I will let you know if it's the case !

DaanDeMeyer commented 4 years ago

@Milerius I've added support for redirecting streams to /dev/null (or equivalent on Windows). You simply specify reproc::options options; options.redirect.out = reproc::redirect::discard; and pass the options to start and all process output is automatically discarded without having to run a separate discard thread.

Milerius commented 4 years ago

@DaanDeMeyer thank's and do we have an equivalent to print the output of every process directly too ?

DaanDeMeyer commented 4 years ago

Yup I added that one as well, reproc::redirect::inherit inherits the parent's stream so for example the stdout of the child process is directly printed to the stdout of the parent process. The same can be done for the stdin and stderr of the child process.

I might add support for redirecting to files as well later.

Milerius commented 4 years ago

@DaanDeMeyer

I think that with reproc::redirect::inherit I can not display the result of my unit tests anymore

without:

Capture d’écran 2019-11-13 à 08 21 00

with:

Capture d’écran 2019-11-13 à 08 22 05

My own logger and output of doctest disappears after the process start.

Ideally I expect the output of my process to appear and my logger to work after launching my process !

Good job otherwise it's seem's exactly what I want !

Milerius commented 4 years ago

Using dup seem's to solve the problem:

26

Capture d’écran 2019-11-13 à 08 39 46

I didn't check if we have the same problem on windows !

DaanDeMeyer commented 4 years ago

I added a Windows implementation that duplicates the handle in redirect. I also did a big refactoring of the redirect function. Can you check if everything still works as expected?

I also broke the API again by hiding the reproc_t implementation in reproc but reproc++'s API is still exactly the same so nothing should break in your case. I also changed the exit_status return type to int but that should be easy to fix (at most a warning).