Re: Race condition seems to be fixed

[Michael Goffioul]

On Mon, May 21, 2012 at 5:26 AM, John W. Eaton <address@hidden> wrote:

On 21-May-2012, Jacob Dawid wrote:

| Hello maintainers,
|
| I probably fixed the race condition a few minutes ago. What I did was to
| separate the workspace model from the view as a prerequisite in the GUI, then I
| added an octave_loop callback into octave's main_loop(), in the same way the
| octave_exit callback was added. Usually, this callback is null, so it won't be
| called when there hasn't been set a callback. I did a hg diff (I hade to modify
| toplev.h and toplev.cc) attached to this mail and kindly ask for confirmation
| that I can push this on the GUI branch, see the very bottom of this file.

Please propose the change to the default branch as a patch separate
from any changes on gui branch.

 diff -r 664f54233c98 src/toplev.cc
 --- a/src/toplev.cc   Sun May 20 22:05:49 2012 +0200
 +++ b/src/toplev.cc   Mon May 21 01:20:34 2012 +0200
 @@ -77,6 +77,7 @@
  #include "variables.h"
  #include "version.h"

 +void (*octave_loop) () = 0;
  void (*octave_exit) (int) = ::exit;

I think we need better names for these function pointers.

  // TRUE means the quit() call is allowed.
 @@ -560,6 +561,9 @@
      {
        try
          {
 +       if (octave_loop)
 +         (*octave_loop) ();
 +
            unwind_protect frame;

            reset_error_handler ();

I'm not sure this is the best place for the callback.  Should it also
be called when Octave is waiting for input?  What about when it is
evaluating a statement passed to the eval function?  Or when Octave is
doing debugging and evaluating expressions at the debug> prompt?

jwe

Jacob,

I'm glad you found a workaround, although I would have liked to pinpoint the real problem, as (proper) atomic refcount should have avoided the race condition.

Regarding your proposal, as John said, it's not generic enough. For instance you're missing any executed readline input hooks. This is typically used in the backends to execute handlers. This design looks very similar to what was done in OctaveDE. At this point, I think your best bet would be to install your own readline input hook. Ideally, this hook should be as light as possible (when there's nothing to be done) to avoid octave consuming CPU in idle state.

Michael.