Boost logo

Boost :

Subject: Re: [boost] Subject: Formal Review of Proposed Boost.Process library starts tomorrow
From: Artyom (artyomtnk_at_[hidden])
Date: 2011-02-10 00:49:36


> > [...] Deadlock Scenario:
> > ------------------
> > 1. I start two asio::io_service instances A, B
> > 2. I create child a and wait for it in A service.
> > 3. I create child b and wait for it in B service.
> > 4. io_service A gets status of children B.
> > 5. service B never terminates as wait would not exit.
> >
> > Race Condition Scenario:
> > ------------------------
> >
> >
> > 1. I start process A and start waiting for its termination
>Asynchronously.
> > 2. I start process B and it terminates
> > 3. wait() receives it status and pid is deleted
> > 4. I wait for B Synchronously.
> > 5. I got a error as PID does not exits.
> >
> >
> > It is badly designed and has major bugs
>
> The only bug I see is then in the documentation where it should be stated
> clearly that this is not and was never intended to be supported (as it's
> indeed impossible with the current implementation).
>

I think if you can't implement this properly just don't implement. IMHO
it is important to have less features that work then half working
onces.

> > It is likely should be implemented by using sigaction on SIGCHLD
> > and allow to actually implement it without additional thread.
>
> [snip]
>
> It's already bad enough that the implementation is so different for Windows
>and POSIX.
> I guess there can be even more implementations as some platforms support some
>system
> calls we could use (like signalfd Bruno mentioned).

Small notice, signalfd has nothing to do with this. It is yet another method
to wait for signal but it still uses signal.

> However I don't think using SIGCHLD is a good idea.
> As a library developer I don't really want to steal the
> signal handler for any signals as we have no idea if there
> are other parts in a program which actually wait for SIGCHLD, too.

SIGCHLD is used for wait operation on the child to exit, it is its
purpose.

So it is better to state in the documentation that it uses SIGCHLD
then to say that

a) Only one io_service can do async waiting
b) You can't use wait in other parts of the code.

Also you don't have to steal the handler you can call it (sigaction returns
previous handler).

> We don't know either if the signal handler is reset after we set it.

No it is not. It is basic knowledge about POSIX/Unix programming.

Also there are more ways to wait for signal, for example if you use
pselect, it exits when signal is delivered (of course you need to use
sigprocmask etc but it is reasonable price for this)

Just study this topic there are lots of things that can be done.

> It should be really the application and not external libraries which manage
> signals. As there can be even more problems like blocked signals using
> Boost.Process could affect a program in ways a developer would probably
> not even think about.
>

I think if you state this clearly in documentation it is OK as
SIGCLD is "Unix" way to wait for child.

Also there are some other tricks but they less portable and error prone
like passing open pipe to the process and wait for it being closed

(It has some other issue as what happens if the process does something
 like for(fd=0;fd<256;fd++) close(fd); etc)

> > 2. The code of thread interruption under POSIX system, it
> > is just no go.
> >
> > void interrupt_work_thread()
> > {
> > // By creating a child process which immediately exits
> > // we interrupt wait().
> > std::vector<std::string> args;
> > args.push_back("-c");
> > args.push_back("'exit'");
> > interrupt_pid_ = create_child("/bin/sh", args).get_id();
> > }
> > a) Why do you assume that "/bin/sh" exists? What I if work
> > in chroot enviroment?
> >
> > calling fork(), exit(1) would be much simpler but still I don't
> > think you should fork at all as overall you should not
> > use "wait()"
> >
> > b) Performance, starting an additonal process to make something
> > to stop is horrible so if I start and stop io_service frequently
> > it would be very slow.
>
> Yes, it's horrible. Still I was glad to interrupt the blocking call to wait()
> at least somehow. Again I wish there was a more suitable POSIX interface I
> could use to emulate an asynchronous wait operation.

POSIX API for this is: sigaction, sigwaitinfo etc.

> But it would probably not make much sense to support
> an asynchronous wait operation for one platform only.
>

Best Regards,

   Artyom

      


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk