Re: Passing variables up to the GUI

[Daniel J Sebald]

On 04/14/2013 07:32 AM, Michael Goffioul wrote:
> On Sun, Apr 14, 2013 at 12:54 AM, Daniel J Sebald
> <address@hidden <mailto:address@hidden>> wrote:
[snip]
> The problem  is that you don't control all of them.
>
>     Here's the scenario:
>
>     In the worker thread is the signal:
>
>
>         retval = eval_string (first_command->toStdString (), false,
>     parse_status, first_nout);
>         if (!parse_status && !error_state)
>           {
>             emit octave_command_result (first_qobject, retval);
>           }
>
>     (could possible include the command string along with the result as
>     a reminder).  Now, if the signal were direct (which it isn't), then
>     Qt wouldn't have to do anything special.  The call by reference
>     could probably break down to a pointer-like operation.  However, if
>     Qt queues the signal (which is the case) then it has to do something
>     to avoid possible loss of that data in the thread.  It will have to
>     copy the data somehow (if it were a pointer instead of reference,
>     then that's a different story); at least that is my guess--and it
>     doesn't have any knowledge about the constructor required to copy
>     the object representation (which constructor would it call?) so it
>     must just do a memcpy of the structural representation of the object
>     (but yes, if there is a member variable that is pointing to a static
>     value, that's potential problems).
>
>
> I don't think that's true. When using queue connection, Qt will make a
> copy of the arguments using the copy constructor, that's part of the
> qRegisterMetaType template stuff. And a copy constructor is a
> requirement for using qRegisterMetaType. Moreover, if you pass an object
> by reference, it'll be treated as pass-by-value if you send it across
> threads [1].

Keeping note of this.  This will be important later.


>     OK, now in the GUI thread I did something like this:
>
>     [snip]::handle_octave_command___result (const QObject* ID, const
>     octave_value_list& ovlres)
>     {
>     [snip]
>            octave_value octval = ovlres (0);
>            if (octval.is_numeric_type ())
>              {
>                Matrix matval = octval.matrix_value ();
>     [snip]
>
>     The rest of the code just accesses the data in matval.  So, if I
>     understand you correctly, the dangerous operation (provided an
>     optimizing compiler doesn't just remove it) is this creation of the
>     octave_value on the stack:
>
>
> There's more than that. ovlres will be copied by Qt on queued
> connection. In the above code, octave_value_list, octave_value and
> Matrix uses reference counting. So any operation on these is dangerous.

Again, taking note, but the signal/slot approach might be OK (i.e., 
maybe).  Will say more...


> We can argue as long as we want about this, it doesn't change the fact
> that there's a fundamental concurrency issue on the reference counting:
> it is not thread-safe. This can be avoided by compiling octave with
> --enable-atomic-refcount.

Just trying to think this through, to avoid going down a coding path 
that seems a bit clumsy right now.

Here is the critical hunk of code:

  octave_value (const octave_value& a)
    {
      rep = a.rep;
      rep->count++;
    }

The issue with the little prototype I did isn't the rep count.  I think 
that is fine.  If Qt signal makes a copy of the ov object in the Worker 
thread, it will increase the count which means the representation for 
that object will still exist no matter if the Worker thread continues 
onward and calls the octave_value destructor.  The count will always be 
at least one...until the slot distribution is completed, at which point 
Qt calls the destructor and then possibly the count comes back down to 
zero.  (Again, this depends on how and where the copy/destruct is done 
by Qt, but I can't imagine that the copy isn't done when signal is 
emitted and the destruct isn't done when the slots are complete.)

I think a similar argument holds for the use of Matrix inside the slot.

The issue is that copying of an octave_value is not actually copying.  The

rep = a.rep;

line means that a copy of an object is pointing at the same data that 
the original is.  So, even though the representation may not go out of 
memory when in the GUI thread slot, the danger is that the Worker thread 
could be modifying that data (size, values, whatever) interspersed when 
the GUI thread is doing the same.  That is the danger.  It would be an 
awful timing bug in which things that normally seem to operate fine in 
the GUI all of a sudden crash.

So the question is, will the Worker thread be modifying the data at the 
same time that the GUI thread could be copying individual elements, 
i.e., just using the octave_value, not actually copying it?  Possibly 
the answer is "no".  Or maybe it is easy to make that the case.  It 
seems to me if a locally constructed octave_value can be made in the 
Worker thread, then emited, the local destructor will happen, then later 
the slot destructor will happen.  That is

1) Create a "local" duplicate of the octave_value in the routine that 
processes the desired command, on the stack or via "new" or whatever. 
That has a rep count of 1.

2) Emit the signal that creates a copy of that local octave_value, 
thereby increasing the rep count to 2.

3) Destroy the local copy off the stack or via "delete".  That brings 
the rep count back down to 1.  *And* from this point forward there is no 
code inside the Worker thread that will touch the representation data of 
the octave_value object because it has gone out of scope to any 
remaining code in the thread.

4) Sometime later the slots are processed in the GUI thread and that 
representation data still exists, unaltered by anything in the Worker 
thread.

5) All slots finish, and Qt calls the destructor for the octave_value. 
The representation count goes down to 0 and the data is deleted from memory.

John, in this sequence of code, where the commands are "size(x);" or 
"x;" where the nout = 1, is there a "duplication" of the octave_value 
(i.e., new data with rep count of 1), or is there only a "copy" of the 
octave_value (i.e., an increase in the rep count)?

    octave_value_list retval;
    int parse_status;
    retval = eval_string (first_command->toStdString (), false, 
parse_status, first_nout);
// What is rep count of retval right here?
    if (!parse_status && !error_state)
      {
// I'm guessing that rep count will be increased by 1 with this
// queued signal.
        emit octave_command_result (first_qobject, retval);
      }

Dan