@jwe: bug #58678 suggests that using BlockingQueuedConnection to initialize and finalize objects is not safe. Could the copy-on-write approach you talked about in https://savannah.gnu.org/bugs/index.php#comment44 help solving this?

For reminder, the reason why I chose to synchronize threads this way are:

• qt_graphics_toolkit::print_figure: we want to be sure the printout file is ready when drawnow returns. This is necessary e.g. for:

plot (1:10)
print -dpdflatexstandalone toto
system "pdflatex toto"

• qt_graphics_toolkit::get_pixels: this function must return pixels from the GUI thread where they are drawn

• qt_graphics_toolkit::finalize: we want to make sure that the reference to the graphics objects held by the GUI Object is released when we return. This way we can use the graphics destructor of the graphics_object to trigger some cleanup actions like with uicontextmenu (see bug #58403)

• qt_graphics_toolkit::initialize: we want to be sure the GUI Object has been created when one of the above functions is called otherwise this bug can happen (invokeMethod fails).

I think first and second points can be circumvented by drawing on an FBO in the interpreter thread.

@Markus: Sorry, looks like I have sent a buggy patch. With the patch from comment #58 GUI objects fail to initialize due to a missing argument to create_object. I'll provide an updated patch when I get some time.

If this is the first deadlock in a while and it happens in the middle of a test which creates graphics objects ... then I would be astonished if it was not provoked by the changes in  cset a379987a74b0.

Another way to have a deadlock (apart from the one in comment #56) is if we use a BlockingQueuedConnection to execute a slot in the same thread as the signal [1].

When initializing the qt_graphics_toolkit (in graphics-init.cc) we use QObject::moveToThread to force the thread affinity of the qt_graphics_toolkit, but we don't check it has taken effect at the time we initialize our first Object.

From Qt's doc on QObject::moveToThread [1]:

>> A QEvent::ThreadChange event is sent to this object just before the thread affinity is changed.

 
The attached cset makes sure a deadlock due to wrong thread affinity cannot happen, but doesn't fix the fact that the qt_graphics_toolkit may not be ready (attached to the GUI thread) when the first object initialization is requested.


[1] https://doc.qt.io/qt-5/qt.html#ConnectionType-enum
[2] https://doc.qt.io/qt-5/qobject.html#moveToThread

(file #49222)

>> Is there any chance that using this method can cause a deadlock?

The only way I know of to provoke a deadlock is if we block the interpreter thread while it owns the graphics mutex, and we try to lock the mutex from the GUI thread.

We should thus make sure, prior to executing methods in the GUI thread, that the interpreter thread does not own the graphics mutex:

• In qt_graphics_toolkit::initialize and qt_graphics_toolkit::finalize we have this construct:

    // FIXME: We need to unlock the mutex here but we have no way to know if
    // if it was previously locked by this thread, and thus if we should
    // re-lock it.

    gh_manager& gh_mgr = m_interpreter.get_gh_manager ();

    gh_mgr.unlock ();

    ...

• As for qt_graphics_toolkit::print_figure, it is currently only called from Fdrawnow after explicitly unlocking the graphics mutex.

          gh_mgr.unlock ();

          go.get_toolkit ().print_figure (go, term, file, debug_file);

          gh_mgr.lock ();

• And finally qt_graphics_toolkit::get_pixels: it is currently only called from F__get_pixels__ which doesn't lock the mutex.

We need to harmonize this all.

I think it would be easier to avoid the construct from Fdrawnow, where the caller has to know that qt_graphics_toolkit::xxx_method needs to be executed while the mutex is unlocked. Instead, the caller should just expect qt_graphics_toolkit::xxx_method to do its job and return with the state of the mutex unchanged.

The problem (see FIXME note in above code) is that the gh_manager does not seem to provide a way to know if the mutex is currently locked by this (interpreter) thread.

I pushed the patch on stable and merged to default:

http://hg.savannah.gnu.org/hgweb/octave/rev/a379987a74b0
http://hg.savannah.gnu.org/hgweb/octave/rev/4ea54a38c500

Very encouraging, thanks to everyone for all the hard work investigating, fixing, and testing this.

I was curious about the history, so I thought I'd document this here in case it's useful later.

This bug was first fixed and closed in December 2018 with the same fix as bug #55226 (https://hg.savannah.gnu.org/hgweb/octave/rev/5535267e88ba), and that fix is part of version 5.2.0.

This bug then reappeared in August 2019 with the refactoring of the qt graphics toolkit (https://hg.savannah.gnu.org/hgweb/octave/rev/ae53e56e16f2).

It currently affects both the stable and default branches, but no tagged releases yet.

I wasn't affected by the original bug. So I don't know if any testing on my part is of much help.
But I went ahead anyway and applied your patch on top of hg id 28676df1deaf locally.
After compiling with it, I ran the dump_plot_demos script from the GUI with the qt toolbox. I thought that this might be a good test because it creates and destroys a lot of graphics objects. (Please let me know if it doesn't test the changes.)
With this test, there don't seem to be any negative side effects on my side.

I'll try with a Windows build next. But - as always - it might take a little to cross-compile.

As for the question of using "error" or "warning_with_id": I'm probably not the most qualified person to comment. But I believe to remember that issues with emitting errors in the GUI thread have been fixed in the past. So I think it should be ok to raise errors in the GUI thread in general.
Also IIUC, having an invalid "m_object" in the ObjectProxy will most likely cause problems later on anyway. So erroring out early sounds reasonable to me.
However, if you are in doubt, maybe it would be safest to push the changes using "warning_with_id" to stable, merge to default and push another change to replace "warning_with_id" with "error" on default. (Or push the patch as is to default, graft to stable and replace "error" with "warning_with_id" on stable.)

It was probably me, trying to fix the crash on exit problem.

Probably it would be good to add some comments explaining why we need special connection types any time we don't use the defaults (I need to remember to do that as well).

confirmed

@Hg200: You are right, the slot never gets called.

I drew my previous erroneous conclusions while using a patch (attached as tst_print.patch) that changes the way we execute the slots. 

I used the following for testing

rm -rf /tmp/t1/*
rm -rf /tmp/tst_print.log
parallel -j 32 -N0 -q ./run-octave --norc --silent --no-history --eval 'figure ("visible", "off"); plot ([1,2]); pipeline = ["/tmp/t1/file-" num2str({#}) ".ps"]; drawnow ("eps", pipeline);' ::: {1..256} > /tmp/tst_print.log 2> /dev/null
ls -c /tmp/t1/ | wc -l

With this patch I see that:

• the ObjectProxy pointer is always valid in qt_graphics_toolkit::print_figure (so I was still wrong): "failed "is never printed
• sometimes "Sending print" (and the rest) is not printed: so it means that the problem is that the proxy is not fully constructed and doesn't hold a valid Figure Object (m_object member).

My conclusion is that making the initialization of ObjectProxy synchronous would probably solve this, at least it does for me with the second attached patch.


(file #49171, file #49172)

With the help of the code from comment #41, I was able to reproduce the bug repeatedly.

The problem comes from this construct:

qt_graphics_toolkit::print_figure (const graphics_object& go,
                                     const std::string& term,
                                     const std::string& file_cmd,
                                     const std::string& /*debug_file*/) const
  {
    ObjectProxy *proxy = toolkitObjectProxy (go);

    if (proxy)
      proxy->print (QString::fromStdString (file_cmd),
                    QString::fromStdString (term));
  }

Sometimes toolkitObjectProxy returns a nullptr and the print operation is silently skipped without error.

You just open it in text editor...
It looks OK. I also realized that we pipe it through ghostscript.
So I changed the output file to svg.
The problem is still there. With gnuplot I get 128 files in 2.7 sec; with opengl I got 92 in 13.3 sec.

Dmitri.
--

Are you sure you are using OpenGL backend when you run the test?
(You can inspect the header for the ps output files to make sure).
If I unset DISPLAY (so the gnuplot backend would be used, then
it makes all 128 files as intended; also it runs about 2x faster.

Dmitri.
--

If it is buffering then serial build would fail as well. It succeeds. My pure speculation is that this is a race condition
with access to GL buffer.

Dmitri.
--

See also bug 56952 (recent comments).
Essentially:

rm -rf /tmp/t1/*

parallel -j 32 -N0 -q ./run-octave --norc --silent --no-history --eval 'plot (1:2); print(tempname("/tmp/t1", "t1-"));' ::: {1..128}

ls -c /tmp/t1/ | wc -l
88

(I expected it to be 128)

Dmitri.
--

@Mike: I am not able to reproduce this reliably (only once in many trials) and have no idea how to debug this.

When printing to EPS and PDF formats we essentially produce an eps contents, using gl2ps, which we pipe to epstool or ghostscript for final adjustment/conversion.

Note that epstool also calls Ghostscript in the background, so I guess that in certain conditions Ghostscript fails to produce the output file and doesn't report an error.

This still affects me on the default branch (Octave 7) today:

+verbatim+
-verbatim-
$ make all
...
  GEN      doc/interpreter/extended.eps
  GEN      doc/interpreter/gplot.eps
  GEN      doc/interpreter/grid.eps
...
  TEXI2DVI doc/interpreter/octave.dvi
/usr/bin/texi2dvi: etex exited with bad status, quitting.
...
$ ls -l doc/interpreter/[eg]*.eps
-rw-r--r-- 1 mike mike  19563 Feb 17 12:51 doc/interpreter/errorbar.eps
-rw-r--r-- 1 mike mike 655555 Feb 17 12:51 doc/interpreter/griddata.eps
-rw-r--r-- 1 mike mike  25608 Feb 17 13:04 doc/interpreter/grid.eps
$
-verbatim-

Building again repeatedly eventually builds all .eps image files and octave.dvi is then built successfully.

It seems like it's the latter for me, that octave.dvi always fails to build.

Attached is the verbose log.

(file #47870)

I'm not sure. I don't see any message about an EPS file not existing. It's basically just

  TEXI2DVI doc/interpreter/octave.dvi
/usr/bin/texi2dvi: etex exited with bad status, quitting.

`texi2dvi --version` gives `6.6`

I experienced the same error with both `-j2` and `-j1`, cf. https://octave.1599824.n4.nabble.com/Problem-building-Octave-td4694553.html

This bug still affects me. Right now, this is very highly reproducible for me, and it seems highly correlated to building with "make -jN". However, I am still sometimes able to reproduce this with serial "make -j1".

Here are some examples from my current build tree, using a sampling of image files that happened to fail to build for me:

## parallel build

$ rm -f doc/interpreter/{convhull,errorbar,hist,mesh,polar}.eps
$ make -j8 doc/interpreter/{convhull,errorbar,hist,mesh,polar}.eps
  GEN      doc/interpreter/convhull.eps
  GEN      doc/interpreter/errorbar.eps
  GEN      doc/interpreter/hist.eps
  GEN      doc/interpreter/mesh.eps
  GEN      doc/interpreter/polar.eps
$ ls -l doc/interpreter/{convhull,errorbar,hist,mesh,polar}.eps
ls: cannot access 'doc/interpreter/errorbar.eps': No such file or directory
ls: cannot access 'doc/interpreter/mesh.eps': No such file or directory
ls: cannot access 'doc/interpreter/polar.eps': No such file or directory
-rw-r--r-- 1 mike mike 17553 Oct 16 11:29 doc/interpreter/convhull.eps
-rw-r--r-- 1 mike mike 13134 Oct 16 11:29 doc/interpreter/hist.eps

## serial build

$ rm -f doc/interpreter/{convhull,errorbar,hist,mesh,polar}.eps
$ make -j1 doc/interpreter/{convhull,errorbar,hist,mesh,polar}.eps
  GEN      doc/interpreter/convhull.eps
  GEN      doc/interpreter/errorbar.eps
  GEN      doc/interpreter/hist.eps
  GEN      doc/interpreter/mesh.eps
  GEN      doc/interpreter/polar.eps
$ ls -l doc/interpreter/{convhull,errorbar,hist,mesh,polar}.eps
ls: cannot access 'doc/interpreter/errorbar.eps': No such file or directory
-rw-r--r-- 1 mike mike  17553 Oct 16 11:31 doc/interpreter/convhull.eps
-rw-r--r-- 1 mike mike  13134 Oct 16 11:31 doc/interpreter/hist.eps
-rw-r--r-- 1 mike mike 839976 Oct 16 11:31 doc/interpreter/mesh.eps
-rw-r--r-- 1 mike mike  19259 Oct 16 11:31 doc/interpreter/polar.eps

As you can see, Octave does not report an error when the command fails to produce a .eps file, and it exits with success, so there is no indication to make that anything failed. The error is caught later when octave.dvi fails to build.

On a second look, this is just your build directory, so the problem must be somewhere else...

Dmitri.
--

I'm hitting the same error as Stefan now in my last build

../doc/interpreter/macro
s.texi:51: Macro name seealso already defined.
@macroxxx ...@the @macname @space already defined}
                                                  @fi @global @cslet {macsav...
l.51 @macro seealso {args}

? ^C/home/mike/src/octave/default/+build-default-sundials4/../doc/interpreter/macro
s.texi:51: Interruption.
<to be read again>
                   @global
@macroxxx ...e @space already defined}@fi @global
                                                  @cslet {macsave.@the @macn...
l.51 @macro seealso {args}

?

Aside from the top-level error being the same "/usr/bin/texi2dvi: etex exited with bad status, quitting.", this does not look like the same bug that this report is about.

With -j1 V=1 I now get this:

'''
texi2dvi  --build-dir=doc/interpreter/octave.t2d -o doc/interpreter/octave.dvi  \
`test -f 'doc/interpreter/octave.texi' || echo '/home/haawda/paketierung/meine_Pakete/octave-hg/src/octave/build/../'`doc/interpreter/octave.texi
This is pdfTeX, Version 3.14159265-2.6-1.40.20 (TeX Live 2019) (preloaded format=etex)
 restricted \write18 enabled.
entering extended mode

(/home/haawda/paketierung/meine_Pakete/octave-hg/src/octave/build/../doc/interp
reter/octave.texi
(/home/haawda/paketierung/meine_Pakete/octave-hg/src/octave/build-aux/texinfo.t
ex Loading texinfo [version 2019-03-16.20]: pdf, fonts, markup, glyphs,
page headings, tables, conditionals, indexing, sectioning, toc, environments,
defuns, macros, cross references, insertions,
(/usr/local/texlive/2019/texmf-dist/tex/generic/epsf/epsf.tex
This is `epsf.tex' v2.7.4 <14 February 2011>
) localization, formatting, and turning on texinfo input format.)
(/home/haawda/paketierung/meine_Pakete/octave-hg/src/octave/build/../doc/interp
reter/macros.texi
/home/haawda/paketierung/meine_Pakete/octave-hg/src/octave/build/../doc/interpr
eter/macros.texi:51: Macro name seealso already defined.
@macroxxx ...@the @macname @space already defined}
                                                  @fi @global @cslet {macsav...
l.51 @macro seealso {args}
                         
?

I suspect that it happens when computer is heavily loaded. The buildbots running multiple parallel builds when this happens.
I cannot reproduce it if I just re-run make on this computer.

(You can also see that this is not a single occurence:
http://buildbot.octave.org:8010/#/builders/9
)

Dmitri.

It looks like creating and destroying ui objects synchronously was not a definite fix, but rather made this bug less probable.

This shows that the issue was clearly not what I though it was (I was sure this bug could simply not happen anymore). Furthermore the synchronous approach made things slower. I'll work on this when I find some time.

@Dmitri: It is strange that creating txt files (that don't even involve creating a figure) crash Octave. This may be a different issue.

After some month where everythink worked as expected I now encounter this again, building from stable mercurial branch or from develop branch.

The patch for bug #55926 has been pushed. Marking this one ready for test.

The bug.sh also fails for me:
...
51: -rw-rw-r--. 1 10512 Dec 14 14:49 doc/interpreter/plot.eps
52: -rw-rw-r--. 1 10512 Dec 14 14:49 doc/interpreter/plot.eps
53: -rw-rw-r--. 1 10512 Dec 14 14:49 doc/interpreter/plot.eps
54: ls: cannot access 'doc/interpreter/plot.eps': No such file or directory

Dmitri.
--

I wonder if it is related to the problem I see with parallel builds. It seems to be hardware dependent. It is almost 100%
reproduciable on i5-4570 (Intel GPU) and Nehalem Xeon
(with Nvidia GPU though I use software rendering).

In my case it is always a problem with "make -j4" and even
"make -j4 V=1" fixes it. Building with gnuplot (instead of qt)
always works fine as well.

Dmitri.
--

On my system, building the figures for the user manual in the .eps format occasionally fails with no error message and no output file produced. The error only becomes apparent when octave.dvi is built, at which point TeX complains about the missing include file.

This is reproducible, although not 100% of the time, and is not always the same figures. The most recent example on my system:

$ rm -f doc/interpreter/*.{dvi,eps,pdf,png}
$ make -j1 V=1 all
…
…/doc/interpreter/poly.texi:838: Could not open file splinefit2.eps, ignoring it.
…
/usr/bin/texi2dvi: etex exited with bad status, quitting.

And sure enough, the file doc/interpreter/splinefit2.eps is missing. Scrolling up through the build output, I do see the following command

/bin/bash run-octave --norc --silent --no-history --path …/doc/interpreter/ --eval "splineimages ('doc/interpreter/', 'splinefit2', 'eps');"

With no error or indication that the command failed, simply no output file produced.

On a second or third repeat of the same sequence, it may work, or there may be a different set of 1 or 2 images missing.

This is with a real display, Intel integrated graphics, Qt toolkit, Qt 5.11.2, gl2ps 1.4.0.