[Vm-dev] Critical OSPP fix (was: OSProcess and lost SIGCHLD)

David T. Lewis lewis at mail.msen.com
Fri Jun 12 02:26:12 UTC 2015


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
> >>>
> >>
> >
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: VMConstruction-Plugins-OSProcessPlugin.oscog-ThierryGoubier.44.mcz
Type: application/octet-stream
Size: 81477 bytes
Desc: not available
Url : http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20150611/17608a26/VMConstruction-Plugins-OSProcessPlugin.oscog-ThierryGoubier.44-0001.obj


More information about the Vm-dev mailing list