On Fri, Sep 03, 2010 at 11:50:10AM -0700, Ross Boylan wrote:
> ExternalWindowsOSProcess>>value is
> value
> 	"Start the external process"
> 
> 	| procInfo mainThread |
> 	self isNotYetRunning ifTrue:
> 		[procInfo := OSProcess accessor primCommand: self commandLine.
> 		procInfo isNil
> 			ifTrue:
> 				[self initialStdErr nextPutAll: 'cannot execute ', self commandLine; cr.
> 				self exitStatus: #cannotExecuteCommandLine.
> 				"FIXME: Close the OSPipes now, otherwise the image will block on a read"
> 				self closeStreams.
> 				[self complete] fork "defer execution so OSPipes stay in place for now"]
> 			ifFalse:
> 				[self pid: (procInfo at: 3).
> 				self handle: (procInfo at: 1).
> 				mainThread := WindowsThread
> 						threadID: (procInfo at: 4)
> 						handle: (procInfo at: 2)
> 						running: true.
> 				self threads add: mainThread.
>  				self running.
> 				OSProcess thisOSProcess registerChildProcess: self.
> 				"FIXME: Close the initial pipe handles. For now, I have not implemented
> 				passing these to the child, and there is no support yet for nonblocking
> 				Windows OS pipes. Once those are available, this method needs to change
> 				to support."
> 				self closeStreams]].
> Because of the possible race in fork (and Cuis's changing it to return
> nil to avoid the race) I reworked part of the code to
> 		procInfo isNil
> 			ifTrue:
> 				[|proc|
> 				self initialStdErr nextPutAll: 'cannot execute ', self commandLine; cr.
> 				self exitStatus: #cannotExecuteCommandLine.
> 				"FIXME: Close the OSPipes now, otherwise the image will block on a read"
> 				self closeStreams.
> 				proc :=[self complete] newProcess.
> 				proc resume.
> 				proc "defer execution so OSPipes stay in place for now"]
> However, I don't understand what the "defer execution" comment is about.
> The execution doesn't look deferred in the original, which appears to
> start the process right away (if one is unaware of the race condition).
> The comment looks even stranger in the new code.

I don't recall why I needed to put the #complete in a background
process, although it probably had something to do with my incomplete
implementation of pipes on Windows. In any case, the use of #fork should
not be a concern here, as we are not making use of the return value.

> 
> BTW this is Win32 code, and so it's not going to run when I do tests on
> Linux.
> 
> The result of the original "[self complete] fork" is a possible return
> value of the method.  I don't know that anything uses that return value,
> but it seemed safest to assure it was a process and not momentarily nil.
> 
> Ross
>