Re: Locking graphics system - GUI/octave synchronization

[Jaroslav Hajek]

On Fri, Jul 18, 2008 at 6:42 PM, John W. Eaton <address@hidden> wrote:
> On 18-Jul-2008, Michael Goffioul wrote:
>
> | On Fri, Jul 18, 2008 at 6:59 AM, John W. Eaton <address@hidden> wrote:
> |
> | > I'm not sure how to debug it.  What would you have me trace?
> |
> | I'm not sure neither. First I'd look into Fsystem (toplev.cc:line 884)
> | to see if the return status comes from the C call "system" or from
> | something else.
>
> On 18-Jul-2008, Jaroslav Hajek wrote:
>
> | Just built, and can't reproduce. Everything works normally.
>
> OK.
>
> I'm running into the problem on a Debian amd64 system.  I haven't
> tried any other type of system yet.
>
> I cloned my local archive (this is the one I push to at
> http://www.octave.org/hg/octave) with the following commands
>
>  hg clone -r 8127 ~/src/octave octave-8127  ## 85d6296d51e1
>  hg clone -r 8128 ~/src/octave octave-8128  ## 78400fde223e
>
> In my archive, 8128 is the revision number of the "Support for
> backend-to-octave event management" changeset.
>
> Then I configured and built both versions with
>
>  ./autogen
>  ./configure CXXFLAGS=-ggdb3 CFLAGS=-ggdb3 F77=gfortran
>  make
>
> Running 8127:
>
>  octave:1> system ("/bin/bash -c 'exit 0'")
>  ans = 0
>
> Running 8128:
>
>  octave:1> system ("/bin/bash -c 'exit 0'")
>  ans =  11
>
> Looking at this with gdb:
>
>  ./run-octave -g
>  GNU gdb 6.8-debian
>  Copyright (C) 2008 Free Software Foundation, Inc.
>  License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
>  This is free software: you are free to change and redistribute it.
>  There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
>  and "show warranty" for details.
>  This GDB was configured as "x86_64-linux-gnu"...
>  (gdb) r
>  Starting program: /scratch/jwe/build/foobar/octave-8128/src/octave 
> --no-initial-path --path=[...]
>  [Thread debugging using libthread_db enabled]
>  [New Thread 0x2b87eb5c23b0 (LWP 30189)]
>  GNU Octave, version 3.0.0+
>  Copyright (C) 2007 John W. Eaton and others.
>  This is free software; see the source code for copying conditions.
>  There is ABSOLUTELY NO WARRANTY; not even for MERCHANTIBILITY or
>  FITNESS FOR A PARTICULAR PURPOSE.  For details, type `warranty'.
>
>  Octave was configured for "x86_64-unknown-linux-gnu".
>
>  Additional information about Octave is available at http://www.octave.org.
>
>  Please contribute if you find this software useful.
>  For more information, visit http://www.octave.org/help-wanted.html
>
>  Report bugs to <address@hidden> (but first, please read
>  http://www.octave.org/bugs.html to learn how to write a helpful report).
>
>  For information about changes from previous versions, type `news'.
>
>  octave:1>
>  Program received signal SIGINT, Interrupt.
>  [Switching to Thread 0x2b87eb5c23b0 (LWP 30189)]
>  0x00002b87ea3d1690 in read () from /lib/libc.so.6
>  (gdb) b Fsystem
>  Breakpoint 1 at 0x2b87e46a546d: file toplev.cc, line 796.
>  (gdb) c
>  Continuing.
>
>  octave:1> system ("/bin/bash -c 'exit 0'")
>
>  Breakpoint 1, Fsystem (address@hidden, nargout=0) at toplev.cc:796
>  796     octave_value_list retval;
>  Current language:  auto; currently c++
>  (gdb) n
>  798     unwind_protect::begin_frame ("Fsystem");
>  (gdb) n
>  800     int nargin = args.length ();
>  (gdb) n
>  802     if (nargin > 0 && nargin < 4)
>  (gdb) n
>  804         bool return_output = (nargout > 1 || nargin > 1);
>  (gdb) n
>  806         std::string cmd_str = args(0).string_value ();
>  (gdb) n
>  808         system_exec_type type = et_sync;
>  (gdb) n
>  810         if (! error_state)
>  (gdb) n
>  812             if (nargin > 2)
>  (gdb) n
>  832         if (! error_state)
>  (gdb) n
>  840             if (type == et_async)
>  (gdb) n
>  880             else if (return_output)
>  (gdb) n
>  884                 int status = system (cmd_str.c_str ());
>  (gdb) n
>  891                 if (WIFEXITED (status))
>  (gdb) p status
>  $1 = 11
>  (gdb) n
>  894                 retval(0) = status;
>  (gdb) p status
>  $2 = 11
>  (gdb) n
>  896           }
>  (gdb) quit
>  The program is running.  Exit anyway? (y or n) y
>
> If I change main.c to be
>
>
>  #ifdef HAVE_CONFIG_H
>  #include <config.h>
>  #endif
>
>  #include "f77-fcn.h"
>  #include "lo-ieee.h"
>
>  #include "octave.h"
>  #include "stdio.h"
>  #include "stdlib.h"
>
>  int
>  main (int argc, char **argv)
>  {
>    fprintf (stderr, "status = %d\n", system ("/bin/bash -c 'exit 0'"));
>
>    return octave_main (argc, argv, 0);
>  }
>
> in both versions, Octave starts with "status = 0" in the 8127 version
> and "status = 11" in the 8128 version.
>
> Strange.
>
> Running ldd src/octave on both versions shows the same set of
> libraries, but the addresses are different.  For example:
>
>  liblapack.so.3gf => /usr/lib/atlas/liblapack.so.3gf (0x00002b927e551000)
>
> for 8127 and
>
>  liblapack.so.3gf => /usr/lib/atlas/liblapack.so.3gf (0x00002b0f16574000)
>
> for 8128 (I don't see how these addresses can matter).
>
> OK, I'll skip the rest of the debugging I did (eliminating parts of
> the patch until it worked), and just say that the attached patch
> solves the problem for me.  I don't fully understand why.  I tried
> using valgrind to see if there was some memory overwriting problem,
> but the static octave_mutex object in cmd-edit.cc is initiailized and
> the system call that I added in main is called (and returns 11 instead
> of 0) before valgrind reports anything.  I do see the following when I
> quit Octave:
>
> ==11627==
> ==11627== Conditional jump or move depends on uninitialised value(s)
> ==11627==    at 0x61A37D7: octave_mutex::~octave_mutex() (oct-mutex.h:53)
> ==11627==    by 0x61A3894: octave_default_mutex::~octave_default_mutex() 
> (oct-mutex.cc:32)
> ==11627==    by 0x61A3749: octave_mutex::~octave_mutex() (oct-mutex.h:55)
> ==11627==    by 0xB030F74: __cxa_finalize (in /lib/libc-2.7.so)
> ==11627==    by 0x6161525: (within 
> /scratch/jwe/build/foobar/octave-8128/liboctave/liboctave.so)
> ==11627==    by 0x674CA40: (within 
> /scratch/jwe/build/foobar/octave-8128/liboctave/liboctave.so)
> ==11627==    by 0xB030C3F: exit (in /lib/libc-2.7.so)
> ==11627==    by 0x554C87A: clean_up_and_exit(int) (toplev.cc:596)
> ==11627==    by 0x554C93B: Fquit(octave_value_list const&, int) 
> (toplev.cc:624)
> ==11627==    by 0x563391D: octave_builtin::do_multi_index_op(int, 
> octave_value_list const&) (ov-builtin.cc:106)
> ==11627==    by 0x55DE70D: octave_value::do_multi_index_op(int, 
> octave_value_list const&) (ov.cc:1076)
> ==11627==    by 0x5710218: tree_identifier::rvalue(int) (pt-id.cc:86)
>
> so there does seem to be some kind of initialization problem with the
> original code.  I don't see this after making the attached changes.
>
> jwe
>
>
>
> diff --git a/liboctave/oct-mutex.cc b/liboctave/oct-mutex.cc
> --- a/liboctave/oct-mutex.cc
> +++ b/liboctave/oct-mutex.cc
> @@ -127,5 +127,5 @@
>  #else
>   rep = new octave_default_mutex ();
>  #endif
> -  rep->count++;
> +  rep->count = 1;
>  }
> diff --git a/liboctave/oct-mutex.h b/liboctave/oct-mutex.h
> --- a/liboctave/oct-mutex.h
> +++ b/liboctave/oct-mutex.h
> @@ -67,7 +67,7 @@
>
>  protected:
>     explicit octave_mutex (int /* dummy */)
> -       : count (0) { }
> +       : rep (0) { }
>
>  protected:
>     union
>
>

I guess you have 32-bit int, right? If so, then I think I understand
the problem:
when ~octave_mutex() is called on the "pointer" object, and rep->count
is decremented
to zero, `delete rep' calls again the same destructor which test
rep->rep, not rep->count!!
If only count is initialized and rep is wider, then it is easily
possible for count to be zero while rep is nonzero.
The trick of using the same class for both rep and pointer is cute but
took me some time to understand ;)

cheers

Jaroslav



-- 
RNDr. Jaroslav Hajek
computing expert
Aeronautical Research and Test Institute (VZLU)
Prague, Czech Republic
url: www.highegg.matfyz.cz