April White:

> - rolled back to changes where I merged IsBuilt, IsDirty, UseMonoFont
> into a bit-field variable
> - removed the code the passed the job object from the ExecuteOne()
> method to command procedure
> - retooled the jobQueue.Pop() method to be jobQueue.GetNextJob()
> - added jobQueue.CurrentJob()
> - moved the jobQueue.GetNextJob() call from SciTEWin::ProcessExecute()
> back to SciTEBase::ProcessExecute()

   OK, I think this is better but there are still some worries.
Exposing a pointer to the current job outside locking is somewhat
dangerous. It looks like it is currently safe but it would be easy for
changes to the code to break thread safety. I just checked in some
changes that improve thread safety in the current code because of
small additions made over time without thought about possible thread
issues. For CurrentJob(), I'd avoid the possibility of breakage where
easy. For example, there could be a safe accessor to getting the
current job directory that used locking and made a copy of the
directory string for SciTEBase::Execute.

   For IDM_FINISHEDEXECUTE / IDM_JOBS, the lifetime of the job
reference extends through code where it would be dangerous although
the current checks appear to make it safe. Moving any use of job to
before the point where IDM_JOBS joins in makes it safer. Since this is
complex processing, the section could reasonably be moved into a
separate method.

   Neil

_______________________________________________
Scite-interest mailing list
[email protected]
http://mailman.lyra.org/mailman/listinfo/scite-interest

Reply via email to