[Vm-dev] Critical OSPP fix (was: OSProcess and lost SIGCHLD)
Esteban Lorenzano
estebanlm at gmail.com
Fri Jun 12 09:55:33 UTC 2015
btw… I cherry picked the change for oscog branch, but since I do not have rights I cannot push it back (another argument agains multiple separated repositories)
Esteban
> On 12 Jun 2015, at 04:26, David T. Lewis <lewis at mail.msen.com> wrote:
>
> Eliot, Esteban:
>
> Thiery Goubier has identified and fixed a critical bug in OSProcessPlugin. The
> issue was improper use of vfork() in a situation where fork() should be used. I
> apparently made this mistake many years ago, and I can't think of any good excuse
> for it. But I am very happy that Thiery has figured it out and fixed it after all
> this time :-)
>
> The fix is in VMConstruction-Plugins-OSProcessPlugin-dtl.40 and updated trunk
> SVN sources.
>
> It is important that we get this fix into the oscog and Pharo branches of OSPP,
> particularly because Thierry is running a Pharo VM and he needs the updates for
> work that he is doing there.
>
> Thierry provided the fix in the attached MCZ, which is relative to
> VMConstruction-Plugins-OSProcessPlugin.oscog-EstebanLorenzano.43 from the Pharo
> repository. This is derived from the oscog branch of OSPP, maybe a bit out of
> date but that is not important WRT this change.
>
> @Eliot: My suggestion would be to update to methods in UnixOSProcessPlugin method
> category 'primitives - fork and exec' to match the latest in the main branch. Alternatively
> you can copy the logic from #primitiveForkExec into the ##forkAndExecInDirectory: in
> your oscog branch (that method no longer exists in my main branch). The relevant
> snippet is:
>
> pid := self fork. "cCode: 'fork()' "
> pid < 0
> ifTrue: [ self perror: 'fork'.
> ^self primitiveFail ].
> pid = 0
> ifFalse:
> [ "Normal return to Smalltalk - this is the old parent process."
> "Enable the timer again before resuming Smalltalk."
> self cCode: 'setitimer (ITIMER_REAL, &saveIntervalTimer, 0L)'.
> interpreterProxy pop: 10; pushInteger: pid
> "Pop 9 arguments plus receiver, push pid."]
> ifTrue:
> [ "This is the new child process"
> ...
>
> @Esteban: You will probably want to follow Eliot's branch, but you can provide
> an immediate fix for Pharo by using Thierry's MCZ attached.
>
> Dave
>
>
> On Thu, Jun 11, 2015 at 09:08:16PM +0200, Thierry Goubier wrote:
>> Le 11/06/2015 20:07, David T. Lewis a ?crit :
>>> Thank you!
>>>
>>> I am traveling, will follow up as soon as I am able.
>>
>> No worries, it will take a while to percolate to the final users.
>>
>> I'm testing it and I now have a nice result: if you enter a non-existent
>> target directory for the fork, you will get the chdir error message on
>> the stderr pipe inside Pharo :)
>>
>> Thierry
>>
>>> Dave
>>>
>>>> Ok, here is a VMConstruction-Plugins-OSProcessPlugin.oscog mcz... Only
>>>> changes are in UnixOSProcessPlugin>>#forkAndExecInDirectory: . I'm using
>>>> Esteban's version of the plugin as ancestor.
>>>>
>>>> I have an account now on squeaksource, at TppG :)
>>>>
>>>> Thierry
>>>>
>>>> 2015-06-11 16:32 GMT+02:00 Thierry Goubier <thierry.goubier at gmail.com>:
>>>>
>>>>>
>>>>>
>>>>> 2015-06-11 15:35 GMT+02:00 Thierry Goubier <thierry.goubier at gmail.com>:
>>>>>
>>>>>> I'm in the slang, and I have a question:
>>>>>>
>>>>>> Does it makes sense to call a primitiveFail in the child process after
>>>>>> the fork? The parent will never get it, isn't it, and the child will
>>>>>> exit
>>>>>> immediately (and only return its exit status).
>>>>>>
>>>>>> (I suspect I didn't get the message 'non existent directory' because
>>>>>> the
>>>>>> primitiveFailFor: reset errno and hence the perror("chdir") prints
>>>>>> nothing)
>>>>>>
>>>>> Wrong, I got the perror, but couldn't make sense of it.
>>>>>
>>>>> OSProcess should return an error if a fork returns a pid of -1, and
>>>>> never
>>>>> let the process run or read data from the pipes.
>>>>>
>>>>> The question about primitiveFail in the child still holds :)
>>>>>
>>>>> I made the change in the UnixOSProcessPlugin in slang, on the pharo-vm
>>>>> distrib; however, there are differences with your version
>>>>> (VMConstruction-Plugins-OSProcessPlugin.oscog-eem.42), I'll have a look
>>>>> at
>>>>> backporting them.
>>>>>
>>>>> Thierry
>>>>>
>>>>
>>>
>>>
> <VMConstruction-Plugins-OSProcessPlugin.oscog-ThierryGoubier.44.mcz>
More information about the Vm-dev
mailing list