<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">of course :)<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On 12 Jun 2015, at 13:10, Thierry Goubier <<a href="mailto:thierry.goubier@gmail.com" class="">thierry.goubier@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Hi Esteban,<div class=""><br class=""></div><div class="">Can I do a pull request on that one?</div><div class=""><br class=""></div><div class="">There are a few things I'd like to see changed apart from the fork to vfork change, to improve some of the error handling. As the code is now, it is impossible, from Pharo, to make the difference between a git commit with no data to commit and a fail in the chdir.</div><div class=""><br class=""></div><div class="">Thierry<br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">2015-06-12 11:55 GMT+02:00 Esteban Lorenzano <span dir="ltr" class=""><<a href="mailto:estebanlm@gmail.com" target="_blank" class="">estebanlm@gmail.com</a>></span>:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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)<br class="">
<br class="">
Esteban<br class="">
<span class=""><br class="">
> On 12 Jun 2015, at 04:26, David T. Lewis <<a href="mailto:lewis@mail.msen.com" class="">lewis@mail.msen.com</a>> wrote:<br class="">
><br class="">
</span><div class=""><div class="h5">> Eliot, Esteban:<br class="">
><br class="">
> Thiery Goubier has identified and fixed a critical bug in OSProcessPlugin. The<br class="">
> issue was improper use of vfork() in a situation where fork() should be used. I<br class="">
> apparently made this mistake many years ago, and I can't think of any good excuse<br class="">
> for it. But I am very happy that Thiery has figured it out and fixed it after all<br class="">
> this time :-)<br class="">
><br class="">
> The fix is in VMConstruction-Plugins-OSProcessPlugin-dtl.40 and updated trunk<br class="">
> SVN sources.<br class="">
><br class="">
> It is important that we get this fix into the oscog and Pharo branches of OSPP,<br class="">
> particularly because Thierry is running a Pharo VM and he needs the updates for<br class="">
> work that he is doing there.<br class="">
><br class="">
> Thierry provided the fix in the attached MCZ, which is relative to<br class="">
> VMConstruction-Plugins-OSProcessPlugin.oscog-EstebanLorenzano.43 from the Pharo<br class="">
> repository. This is derived from the oscog branch of OSPP, maybe a bit out of<br class="">
> date but that is not important WRT this change.<br class="">
><br class="">
> @Eliot: My suggestion would be to update to methods in UnixOSProcessPlugin method<br class="">
> category 'primitives - fork and exec' to match the latest in the main branch. Alternatively<br class="">
> you can copy the logic from #primitiveForkExec into the ##forkAndExecInDirectory: in<br class="">
> your oscog branch (that method no longer exists in my main branch). The relevant<br class="">
> snippet is:<br class="">
><br class="">
> pid := self fork. "cCode: 'fork()' "<br class="">
> pid < 0<br class="">
> ifTrue: [ self perror: 'fork'.<br class="">
> ^self primitiveFail ].<br class="">
> pid = 0<br class="">
> ifFalse:<br class="">
> [ "Normal return to Smalltalk - this is the old parent process."<br class="">
> "Enable the timer again before resuming Smalltalk."<br class="">
> self cCode: 'setitimer (ITIMER_REAL, &saveIntervalTimer, 0L)'.<br class="">
> interpreterProxy pop: 10; pushInteger: pid<br class="">
> "Pop 9 arguments plus receiver, push pid."]<br class="">
> ifTrue:<br class="">
> [ "This is the new child process"<br class="">
> ...<br class="">
><br class="">
> @Esteban: You will probably want to follow Eliot's branch, but you can provide<br class="">
> an immediate fix for Pharo by using Thierry's MCZ attached.<br class="">
><br class="">
> Dave<br class="">
><br class="">
><br class="">
> On Thu, Jun 11, 2015 at 09:08:16PM +0200, Thierry Goubier wrote:<br class="">
>> Le 11/06/2015 20:07, David T. Lewis a ?crit :<br class="">
>>> Thank you!<br class="">
>>><br class="">
>>> I am traveling, will follow up as soon as I am able.<br class="">
>><br class="">
>> No worries, it will take a while to percolate to the final users.<br class="">
>><br class="">
>> I'm testing it and I now have a nice result: if you enter a non-existent<br class="">
>> target directory for the fork, you will get the chdir error message on<br class="">
>> the stderr pipe inside Pharo :)<br class="">
>><br class="">
>> Thierry<br class="">
>><br class="">
>>> Dave<br class="">
>>><br class="">
>>>> Ok, here is a VMConstruction-Plugins-OSProcessPlugin.oscog mcz... Only<br class="">
>>>> changes are in UnixOSProcessPlugin>>#forkAndExecInDirectory: . I'm using<br class="">
>>>> Esteban's version of the plugin as ancestor.<br class="">
>>>><br class="">
>>>> I have an account now on squeaksource, at TppG :)<br class="">
>>>><br class="">
>>>> Thierry<br class="">
>>>><br class="">
>>>> 2015-06-11 16:32 GMT+02:00 Thierry Goubier <<a href="mailto:thierry.goubier@gmail.com" class="">thierry.goubier@gmail.com</a>>:<br class="">
>>>><br class="">
>>>>><br class="">
>>>>><br class="">
>>>>> 2015-06-11 15:35 GMT+02:00 Thierry Goubier <<a href="mailto:thierry.goubier@gmail.com" class="">thierry.goubier@gmail.com</a>>:<br class="">
>>>>><br class="">
>>>>>> I'm in the slang, and I have a question:<br class="">
>>>>>><br class="">
>>>>>> Does it makes sense to call a primitiveFail in the child process after<br class="">
>>>>>> the fork? The parent will never get it, isn't it, and the child will<br class="">
>>>>>> exit<br class="">
>>>>>> immediately (and only return its exit status).<br class="">
>>>>>><br class="">
>>>>>> (I suspect I didn't get the message 'non existent directory' because<br class="">
>>>>>> the<br class="">
>>>>>> primitiveFailFor: reset errno and hence the perror("chdir") prints<br class="">
>>>>>> nothing)<br class="">
>>>>>><br class="">
>>>>> Wrong, I got the perror, but couldn't make sense of it.<br class="">
>>>>><br class="">
>>>>> OSProcess should return an error if a fork returns a pid of -1, and<br class="">
>>>>> never<br class="">
>>>>> let the process run or read data from the pipes.<br class="">
>>>>><br class="">
>>>>> The question about primitiveFail in the child still holds :)<br class="">
>>>>><br class="">
>>>>> I made the change in the UnixOSProcessPlugin in slang, on the pharo-vm<br class="">
>>>>> distrib; however, there are differences with your version<br class="">
>>>>> (VMConstruction-Plugins-OSProcessPlugin.oscog-eem.42), I'll have a look<br class="">
>>>>> at<br class="">
>>>>> backporting them.<br class="">
>>>>><br class="">
>>>>> Thierry<br class="">
>>>>><br class="">
>>>><br class="">
>>><br class="">
>>><br class="">
</div></div>> <VMConstruction-Plugins-OSProcessPlugin.oscog-ThierryGoubier.44.mcz><br class="">
<br class="">
</blockquote></div><br class=""></div></div></div>
</div></blockquote></div><br class=""></div></body></html>