April White:

> I've uploaded
> to http://www.scintilla.org/aprilw/scite-april-2005-12-29.zip my
> implementation of the new job system for v1.67

   OK. The PropSetFile change is now committed.

> It is fixed; I decided to upload todays revisions under the same name as
> above.

   OK. There are some threading worries. jobQueue.CurrentJob() returns
a pointer to a single job object which could potentially be deleted
while the pointer is being used even though currently this may be
safe. Since nothing is written into this pointer, you could make the
pointers const and also hand out a copy to ensure it can be used
without worrying about deletion.

   JobQueue::CurrentJobDirectory has a small vulnerability since it
doesn't lock and copying a FilePath involves allocations. This could
become something like

FilePath JobQueue::CurrentJobDirectory() const {
    mutex->Lock();
    FilePath ret = currentJobDirectory;
    mutex->Unlock();
    return ret;
}

   Once there are a bunch of synchronized methods, it can be nice to
clean things up with a Lock class using RAII (resource acquisition is
initialisation) like

FilePath JobQueue::CurrentJobDirectory() const {
    Lock lock(mutex);
    return currentJobDirectory;
    // Freeing the lock occurs here so all code in the return
    // statement is protected.
}

   There is an example at
http://www.codeproject.com/threads/cppsyncstm.asp but I wouldn't go so
far as to use their synchronized macro.

   Neil

_______________________________________________
Scite-interest mailing list
Scite-interest@lyra.org
http://mailman.lyra.org/mailman/listinfo/scite-interest

Reply via email to