On 02/09/2010 06:48 p.m., Ross Boylan wrote:
> On Thu, 2010-09-02 at 14:20 -0700, Ross Boylan wrote:
>> I'm getting errors trying to file OSProcess (the st file from
>> OSProcess-dtl.56.mcz) into Cuis.  I am trying to do this so I can run
>> magma tests.
>>
>> I don't know if there's any chance of this working.  In the image
>> Smalltalk listBuiltinModules #('ExuperyPlugin 27 February 2009 (i)')
>> Smalltalk listLoadedModules. #('UnixOSProcessPlugin 27 February 2009 (e)' 'SoundPlugin 27 February 2009 (e)' 'BitBltPlugin 8 April 2009 (e)' 'LargeIntegers v1.5 27 February 2009 (e)' 'MiscPrimitivePlugin 27 February 2009 (e)' 'SecurityPlugin 27 February 2009 (e)' 'FilePlugin 27 February 2009 (e)')
>> So it looks as if the necessary  plugin is there (Smalltalk vmVersion
>> 'Pharo0.1 of 16 May 2008 [latest update: #10074]')
>>
>> I think there's a small bug in OSProcessAccessor
>> class>>forThisOSProcess.  I changed the last 2 lines to be
>> 			oldAccessor ifNotNil:
>> 				[oldAccessor release; finalize.].
>> 			^ ThisOSProcessAccessor := self concreteClass basicNew initialize]
>> I added the ifNotNil: test; without it release fails because it is sent
>> to an undefined object.  The ifNotNil: test probably belongs earlier in
>> the code, but this was enough to get it to run.
>>
>> However, the final initialize fails.  It calls
>> UnixOSProcessAccessor>>grimReaperProcess, which fails because grimReaper
>> is nil.  I notice the very last OSProcess changeset touched this method.
>>
> I think there are 2 separate problems.  First, the parentheses are off.
> I think they should be
> grimReaperProcess
> 	"This is a process which waits for the death of a child OSProcess, and
> 	informs any dependents of the change. Use SIGCHLD events if possible,
> 	otherwise a Delay to poll for exiting child processes."
>
> 	| event processSynchronizationDelay |
> 	^ self canAccessSystem
> 		ifTrue:
> 			[event := (self canAccessSystem and: [self canForwardExternalSignals])
> 				ifTrue: [self sigChldSemaphore]
> 				ifFalse: [Delay forMilliseconds: 200].
> 			processSynchronizationDelay := Delay forMilliseconds: 20.
> 			grimReaper ifNil:
> 				[grimReaper :=
> 					[[event wait.
> 					processSynchronizationDelay wait. "Avoids lost signals in heavy process switching"
> 					self changed: #childProcessStatus] repeat] fork.].
> 			"name selected to look reasonable in the process browser"
> 			grimReaper name: ((ReadStream on: grimReaper hash asString) next: 5)
> 							, ': the child OSProcess watcher']
> 		ifFalse:
> 			[nil]
> I moved a parentheses from after 'OSProcess watcher' on the 3rd to last
> line to after the fork.  I also added "." after the ].
>
> The second problem is that fork in Cuis does not return an object.  This
> is perhaps related to the bug that inspired the latest OSProcess
> release.  There's a very informative comment on the Cuis fork method:
> fork
> 	"Create and schedule a Process running the code in the receiver."
> 	
> 	"jmv - Do NOT answer the new process.
> 	
> 	See http://lists.squeakfoundation.org/pipermail/squeak-dev/2008-February/124960.html
> 	
> 	Most times, this methods returns before resuming the new process (if priority of new process is less
> 	or equal than current). But it might return afterwards.
> 	
> 	This means it is very dangerous to use the returned process in code that stores it in some variable
> 	and checks for nil to start a new one. If this methods happens to return after the new process is forked,
> 	chances are the code that starts all this runs again, that variable is nil, and a second process is forked,
> 	perhaps breaking some shared state. This kind of bug is hard to spot and debug.
> 	
> 	Callers wanting the new process object, should call #newProcess, store the answer, and then #resume.
> 	
> 	A way to ensure this bug will not ever happen again is just to answer nil"
>
> 	self newProcess resume.
> 	^nil
>
> I think the fix is to do that (newProcess and resume), but perhaps this
> means some of the syncronization code in the current grimReaperProcess
> can go.
>
> Ross
>
> P.S. The original error arose when grimReaper name: failed.

I believe it would be better to have #fork and friends return nil also 
in Squeak. It is much easier to fix senders...

Another alternative, even more radical is to have #fork and friends do

self error: 'Never call this method. Use #newProcess and #resume instead'

to make senders easier to spot. You needed to figure yourself that the 
answer of #fork was nil. The argument against this is that there are 
"legitimate" senders of #fork, i.e. code that does not use the returned 
value...

Cheers,
Juan Vuletich