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

David T. Lewis lewis at mail.msen.com
Fri Jun 12 12:07:47 UTC 2015


On Fri, Jun 12, 2015 at 12:04:39PM +0200, Esteban Lorenzano wrote:
> 
> another btw??? 
> 
> right now implementation of primitive is very different in oscog branch and interpreter branch. 
> for now I just took the change of vfork for fork, but??? shouldn???t those branches be more alike? 
> 
> 
> Esteban

The oscog branch of the primitive is out of date. But it also may contain improvements
that I have not yet included in the main OSPP. One of its changes is wrong and causes
the test failure in UnixProcessAccessorTestCase>>tetRedirectStdOutTo. Otherwise they
should be more or less functionally the same.

>From my personal point of view it is a real pain to try to keep track of multiple
branches of the plugin.

Dave

> 
> > On 12 Jun 2015, at 11:55, Esteban Lorenzano <estebanlm at gmail.com> wrote:
> > 
> > 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