August 30
https://issues.dlang.org/show_bug.cgi?id=23654

--- Comment #9 from Steven Schveighoffer <schveiguy@gmail.com> ---
(In reply to anonymous4 from comment #6)
> Doesn't fork break the GC heap? If you want to allocate from GC after fork, it may not work.

fork *can* hang if another thread is holding the GC lock. But we can potentially work around this if we take the GC lock before calling fork.

But using the GC has always been a part of this, it's not new. The toAStringz is using the GC. The problem with the original change is it moved the allocation of the array (which points at GC memory) from a scannable place to a non-scannable place.

--
August 30
https://issues.dlang.org/show_bug.cgi?id=23654

--- Comment #10 from anonymous4 <dfj1esp02@sneakemail.com> ---
As I understand the code initially didn't touch GC (because it can fail after fork or to match posix guarantees) then regressed to use GC.

--
August 30
https://issues.dlang.org/show_bug.cgi?id=23654

--- Comment #11 from Steven Schveighoffer <schveiguy@gmail.com> ---
If you look at that commit from 2015, it was using alloca before, and now uses malloc. But the issue is that the `toAStringz` which allocates an array of C strings using the GC. This was the case before the 2015 change, and is still the case now. So the GC was always used.

The issue that happened when moving to malloc is that the GC could clean up those string items before they were sent to execv. When it was using alloca, that went on the stack, and stacks are scanned.

The simple solution is to put that array on the GC.

Given that for at least 8 years, and probably more, using the GC was fine when forking/execing a process, most likely it's either just fine, or it hangs so infrequently that nobody has complained about it.

Using the GC for the string array along with all the strings is at least the same risk as just allocating the strings using the GC.

--
October 22
https://issues.dlang.org/show_bug.cgi?id=23654

--- Comment #12 from kdevel <kdevel@vogtner.de> ---
(In reply to kdevel from comment #5)
> Created attachment 1868 [details]
> Patch (v0) against process.d
> 
> Could not yet test this code. That is how I would implement those D wrappers.

The test is rather simple.

1. Locate process.d (locate src/phobos/std/process.d) and copy it
   to the current working directory.

2. Download, compile, execute execv-recur.d (4th attachment)

   $ dmd execv-recur.d process.d
   $ ./execv-recur 1
   [...]
   self <./execv-recur> newargs [0, 1] <./execv-recur> <131072>
   dump:  <2> <4> <2> <32> <2> <4> <2>
   object.Exception@execv-recur.d(49): exp <4> actual <131072>
   [...]

3. Download the Patch (3rd attachment) apply, recompile, execute:

   $ patch -i issue-23654.v0.patch
   patching file process.d
   Hunk #1 succeeded at 4309 (offset 50 lines).
   Hunk #2 succeeded at 4374 (offset 50 lines).
   $ dmd execv-recur.d process.d
   $ ./execv-recur 1
   [...]
   self <./execv-recur> newargs [0, 1] <./execv-recur> <262144>
   rc = <-1> errno = <7> strerror = <Argument list too long>

This is the expected outcome.

--
1 2
Next ›   Last »