Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Design consideration] process_waitee #59

Open
klemens-morgenstern opened this issue Oct 11, 2020 · 7 comments
Open

[Design consideration] process_waitee #59

klemens-morgenstern opened this issue Oct 11, 2020 · 7 comments

Comments

@klemens-morgenstern
Copy link
Contributor

klemens-morgenstern commented Oct 11, 2020

Basesd on #57 i would argue, that we need an async_wait functionality.

One design question is if we turn the process into a waitable object, like so:

std::process proc{...};

io_context ctx;
proc.async_wait(ctx, ...);
proc.cancel_async_wait();

The downside is that we need to pass the executor into the async_wait function, which is inconsisten with the networking TS design. Secondly, the process class will need data members that hold the async wait implementation, currently asio::signal_set and object_handle.

Alternatively use an object holding all the asio stuff named process_waitee or something.

std::process proc{...};

io_context ctx;

std::process_waitee w{ctx, proc};
w.async_wait(...);
w.cancel();

The second approach has a clearer design with an io object, but has a sharing problem. That is: in order for the process object to have the right exit_code, the waitee would need to keep a reference to it, or make the internal exit_code a shared_pointer. If not, the process might not only have the wrong exit code, but might also be unaware that the process ended.
Thus the first approach has a slightly unusual function signature, but the second solution introduces a bunch of complexity.

@klemens-morgenstern klemens-morgenstern changed the title [Design consideration] [Design consideration] process_waitee Oct 11, 2020
@klemens-morgenstern
Copy link
Contributor Author

An additional feature of the second approach is that we could remove this from the initial paper, since it's just another class. I'll give it a try in my reference implementation.

@klemens-morgenstern
Copy link
Contributor Author

OTH: There has to be clear ownership of the process object, because otherwise there might be a mismatch on the exit_code. Having a seperate object might introduce a few sharp edges.

@JeffGarland
Copy link
Owner

JeffGarland commented Oct 11, 2020

On a related note, I don't know if you've been following this but this paper is the currently agreed design for executors that is being pushed for c++23 related to the networking TS. I don't know how much of it is implemented as of yet by Chris (I know he's tracking it) or others. http://wg21.link/p0443 -- anyway we need to fit into this.

@klemens-morgenstern
Copy link
Contributor Author

I just added async_wait as a member like this: https://github.com/klemens-morgenstern/process/blob/master/test/wait_exit.cpp#L33

I think we can leave it for now, but maybe mention the alternative when we discuss this.

@klemens-morgenstern
Copy link
Contributor Author

I have been marginally aware of this paper, but the nice thing is that we only really have to design the async_wait. The pipes are basically a copy of any streaming IO object like tcp::sockets so that we don't need to design much there.

@JeffGarland
Copy link
Owner

@klemens-morgenstern where would we find the writeup?

@klemens-morgenstern
Copy link
Contributor Author

Arrg, I responed to the wrong email, so that went to github. Will mail it out again in a minute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants