On Fri, 07 Jun 2013 01:59:22 -0400, Lars T. Kyllingstad
<public@kyllingen.net> wrote:

> On Thursday, 6 June 2013 at 17:32:25 UTC, nazriel wrote:
>> I am aware that std.process is generalized but I doubt such useful functionality which is usable on various Posixen is more disturbing than Windows-only suprpressConsole https://github.com/D-Programming-Language/phobos/blob/master/std/process.d#L954
>
> I think there is a huge difference between a simple flag and the
> ability to execute arbitrary code on one OS but not on another.
> (When set, suppressConsole actually *eliminates* a difference in
> the default behaviour of the two OS families.)

First, suppressConsole is a simple flag, basically passed through to the
CreateProcess function, so even though it's Windows-specific, so is the
behavior we are suppressing.  Consider that when you specify
suppressConsole on Posix, the flag works!  No console window is created :)

But what to do with arbitrary "run this code between fork and exec" on
windows?  It's not possible.  It doesn't belong in the generalized API.

>> But I was mistaken. Config is an enum not struct, so yeah, not worth changing it only for sake of posix callback.
>>
>> So maybe module level variable?
>>
>> module std.process;
>>
>> // ...
>> void delegate() posixPostFork = null;
>> // ...
>
> Global state?  Don't want to go there...

I agree that the global state is a bad idea, ideally you want to specify
PER CALL what happens on a fork/exec, not PER THREAD (or PER PROCESS).

But I think we need some way to hook this.  To give up all the niceties of std.process just so you can hook the fork/exec sequence seems overly burdensome.

What I am thinking of is possibly to expose the OS-specific spawnProcess implementation as an object with the API defined by it, similar to how writeln simply forwards to stdout.writeln.  We could have spawnProcess simply forward to posixProcessImpl.spawnProcess (or windowsProcessImpl.spawnProcess on windows)

Then if someone wants to actually take advantage of OS-specific features, they can call on the appropriate object.  It shouldn't compile where it's not implemented (e.g. windows spawnProcess shouldn't be callable on Linux).

Does this make sense?  I think it can be done without breaking any code.  May be a lot of boilerplate :)

-Steve

On Monday, 10 June 2013 at 16:20:53 UTC, Steven Schveighoffer wrote:
> But I think we need some way to hook this.  To give up all the niceties of std.process just so you can hook the fork/exec sequence seems overly burdensome.

But with the pthread_atfork() solution you don't have to.  Call that function before you call spawnProcess() or any of the other std.process functions, and it should Just Work (TM).  The nice thing here is that it is already part of the POSIX standard, and thus should be available on all relevant systems, and we don't have to adapt our cross-platform API at all.

On Mon, 10 Jun 2013 16:05:15 -0400, Lars T. Kyllingstad <public@kyllingen.net> wrote:

> On Monday, 10 June 2013 at 16:20:53 UTC, Steven Schveighoffer wrote:
>> But I think we need some way to hook this.  To give up all the niceties of std.process just so you can hook the fork/exec sequence seems overly burdensome.
>
> But with the pthread_atfork() solution you don't have to.  Call that function before you call spawnProcess() or any of the other std.process functions, and it should Just Work (TM).  The nice thing here is that it is already part of the POSIX standard, and thus should be available on all relevant systems, and we don't have to adapt our cross-platform API at all.

This is not a good solution.  It deals with the idea that when forking, only the calling thread is alive, all other threads are dead, and one of those dead threads may hold a lock.  Also note that the function pointers are function pointers, not delegates.

The idea is that prior to fork, you lock all mutexes you want to be unlocked.  Then after fork is called, you unlock those mutexes (thus ensuring no dead threads hold the locks).

I don't think it makes for a very good generalized solution to "I want to run this arbitrary code".

Also, according to SO, it doesn't even do what it means to do, since the newly created process thread can't unlock the mutexes:

http://stackoverflow.com/questions/2620313/how-to-use-pthread-atfork-and-pthread-once-to-reinitialize-mutexes-in-child

Not only that, but it seems to be permanent -- there is no "unregister pthread_atfork" call.  So this has to be a one-time *process-wide* and permanent solution.  If you wanted to run code for this specific call to spawnProcess, and not others, then you are SOL.

And finally, if your ultimate purpose is to call exec right after fork (as it is in the general case), you are penalized by having to wait for some mutex to be unlocked in order to fork.

-Steve

On Monday, 10 June 2013 at 20:26:59 UTC, Steven Schveighoffer wrote:
>
> This is not a good solution.
> [...]

Ok, you make many good points that I hadn't even thought about.  I admit, I didn't look too hard at the phtread_atfork() documentation after I'd decided it was just the thing. ;)  So maybe we have to do something about this after all.

I'll comment on your earlier post.

On Monday, 10 June 2013 at 16:20:53 UTC, Steven Schveighoffer wrote:
> What I am thinking of is possibly to expose the OS-specific spawnProcess implementation as an object with the API defined by it, similar to how writeln simply forwards to stdout.writeln.  We could have spawnProcess simply forward to posixProcessImpl.spawnProcess (or windowsProcessImpl.spawnProcess on windows)
>
> Then if someone wants to actually take advantage of OS-specific features, they can call on the appropriate object.  It shouldn't compile where it's not implemented (e.g. windows spawnProcess shouldn't be callable on Linux).

Why should we add an object?  Why not expose the OS-specific spawnProcess() implementation as it is -- a free function -- with an additional parameter which specifies a callback delegate?

Can you think of any other places we'd want to hook besides post-fork-pre-exec?

On Tue, 11 Jun 2013 12:31:19 -0400, Lars T. Kyllingstad <public@kyllingen.net> wrote:

> On Monday, 10 June 2013 at 16:20:53 UTC, Steven Schveighoffer wrote:
>> What I am thinking of is possibly to expose the OS-specific spawnProcess implementation as an object with the API defined by it, similar to how writeln simply forwards to stdout.writeln.  We could have spawnProcess simply forward to posixProcessImpl.spawnProcess (or windowsProcessImpl.spawnProcess on windows)
>>
>> Then if someone wants to actually take advantage of OS-specific features, they can call on the appropriate object.  It shouldn't compile where it's not implemented (e.g. windows spawnProcess shouldn't be callable on Linux).
>
> Why should we add an object?  Why not expose the OS-specific spawnProcess() implementation as it is -- a free function -- with an additional parameter which specifies a callback delegate?

We could do that too.  The only thing is that the implementation function is more rough -- it doesn't have all the nice overloads.  I was thinking of simply moving the overloads to an object, and then calling on the object's overloads.

But thinking about it now, it doesn't make sense.  The object's overloads would all have to support this callback parameter.  It's probably best to simply expose the underlying implementation.

The documentation should explicitly warn about this...  From reading the pthread_atfork man page, it is clear that there are very significant problems that can arise from running arbitrary code at that time.

> Can you think of any other places we'd want to hook besides post-fork-pre-exec?

I can't think of anything else at the moment.  If we ever support async events in Phobos, we should add an event for "child exited".

-Steve

On Monday, 10 June 2013 at 16:20:53 UTC, Steven Schveighoffer wrote:
> On Fri, 07 Jun 2013 01:59:22 -0400, Lars T. Kyllingstad
> <public@kyllingen.net> wrote:
>
>> On Thursday, 6 June 2013 at 17:32:25 UTC, nazriel wrote:
>>> I am aware that std.process is generalized but I doubt such useful functionality which is usable on various Posixen is more disturbing than Windows-only suprpressConsole https://github.com/D-Programming-Language/phobos/blob/master/std/process.d#L954
>>
>> I think there is a huge difference between a simple flag and the
>> ability to execute arbitrary code on one OS but not on another.
>> (When set, suppressConsole actually *eliminates* a difference in
>> the default behaviour of the two OS families.)
>
> First, suppressConsole is a simple flag, basically passed through to the
> CreateProcess function, so even though it's Windows-specific, so is the
> behavior we are suppressing.  Consider that when you specify
> suppressConsole on Posix, the flag works!  No console window is created :)
>
> But what to do with arbitrary "run this code between fork and exec" on
> windows?  It's not possible.  It doesn't belong in the generalized API.
>
>>> But I was mistaken. Config is an enum not struct, so yeah, not worth changing it only for sake of posix callback.
>>>
>>> So maybe module level variable?
>>>
>>> module std.process;
>>>
>>> // ...
>>> void delegate() posixPostFork = null;
>>> // ...
>>
>> Global state?  Don't want to go there...
>
> I agree that the global state is a bad idea, ideally you want to specify
> PER CALL what happens on a fork/exec, not PER THREAD (or PER PROCESS).
>
> But I think we need some way to hook this.  To give up all the niceties of std.process just so you can hook the fork/exec sequence seems overly burdensome.
>
> What I am thinking of is possibly to expose the OS-specific spawnProcess implementation as an object with the API defined by it, similar to how writeln simply forwards to stdout.writeln.  We could have spawnProcess simply forward to posixProcessImpl.spawnProcess (or windowsProcessImpl.spawnProcess on windows)
>

I had no time yet to check out the pthread_atfork approach but I see you found some issue with that.

posixProcessImpl.spawnProcess and windowsProcessImpl.spawnProcess sounds very good.

> Then if someone wants to actually take advantage of OS-specific features, they can call on the appropriate object.  It shouldn't compile where it's not implemented (e.g. windows spawnProcess shouldn't be callable on Linux).
>

That would be great.
Being able to use the easy, simple functions to get the work done and encourage to use them (by Docs and examples) but also allow access to more specialized, "less visible" functions as you proposed.

> Does this make sense?  I think it can be done without breaking any code.  May be a lot of boilerplate :)
>
> -Steve

Thank you very much for looking into this.