Re: [PATCH] Thread-safe graphics manager

[Michael Goffioul]

On Sat, Jul 5, 2008 at 11:59 AM, Maciek Gajewski
<address@hidden> wrote:
> Michael Goffioul wrote:
>> Do you really need to add locking at all these places? Isn't it enough
>> to add locking at the entry-point the user has access to, meaning the
>> DEFUN_DLD functions? In any case, octave will access the graphics
>> system through one of these function, right?
>
> Yes and no. Indeed, all user interaction goes through DEFUN_DLD functions,
> but:
> - something may be called by function registered with input_event_hook,
> - something may be called by multi-threaded backend from different thread.

That's the point: on one hand, the interpreter is protected if you use locking
in the DEFUN_DLD functions; on the other hand any code that uses
octave graphics system is responsible for locking it. Adding some
additional protection like you did is harmless, but it must be consistent
(see below).

> Of course in both cases programmer should be aware of locking problem and lock
> explicitly, but since locks came out to be cheap, I've added them whenever
> they seemed useful.

You've added a lock in a very few get/set handlers, basically only those
defined in graphics.cc. But most of the get/set handlers are auto-generated,
and those are not protected. So, I would say either you protect them all,
or you don't protect any. If you want to protect them all, then you might
a problem due to the fact that all get/set auto-handlers are defined inline
in the generated graphics.h and the class gh_manager is only defined
afterwards.

Michael.