Re: Thread-safety issues in QtHandles

[Michael Goffioul]

On Mon, Oct 31, 2011 at 4:27 PM, John W. Eaton <address@hidden> wrote:
> OK, maybe this is why Jaroslav created the octave_refcount class.  He
> never explained that to me.  It might have been that he wanted to move
> in this direction but while preserving the interface in which we still
> have a count object.

Maybe this is the case. But to ensure thread safety,
incrementing/decrementing the refcount must be an atomic operation at
CPU level. I suppose the implementation is highly architecture
dependent. As this problem has already been solved in shared_ptr, I
think it's better to use it instead of trying to re-invent the wheel.
That's why I started to look at shared_ptr.

> Thanks, seeing the example helped.
>
> Here is a simplified version of what I think we want to do, if
> possible.  This example program does not work properly.  See the
> comment in foo::foo (foo_rep *new_rep, bool borrow) constructor.  This
> constructor is intended to serve the same purpose as the
> octave_value::octave_value (octave_base_value *new_rep, bool borrow)
> constructor in the octave_value class.  I think that constructor could
> have been used in place of the lines like
>
>  count++
>  retval = octave_value (this);
>
> that you see in files like ov-struct.cc.
>
> If it is not possible to grab an extra reference with the shared_ptr
> interface, is there some other way to do this and preserve the current
> interface in which subsasgn returns an octave_value object?  If so,
> could you modify the foo::doit and foo_rep::doit methods in the
> example below to show how that would work?

See attachment. Note that I modified foo::doit as I think it was a
mistake to return "*this". The pattern as shown in the attached file
is what I'm currently trying with octave_value/octave_base_value. For
consistency/safety, I also changed the signature of functions
returning octave_base_value* (like clone, unique_clone...). Of course,
this impacts any class inheriting from octave_base_value.

> Note that the change you are proposing will affect more than just
> code in Octave.  There are some classes in Octave Forge that will be
> affected because they also use the count member directly.  I've never
> particularly liked the fact that classes had to increment count (or
> ask the constructor to increment it), so this may have been bad
> design.  I'm not opposed to fixing it, but there will be some
> breakage if we delete the count member from the octave_value class.

I know that this will basically break everything... Though there have
also been significant changes in 3.4 release that broke some
octave-forge package.

Note that my goal is to provide make octave more thread-safe and solve
the issues I have in QtHandles. Once I've completed the changes, I'll
test them in QtHandles and check whether the issues are actually
solved. If there are not, there will probably be no point in
integrating those changes in octave.

Michael.

test_sharedptr2.cc
Description: Binary data