#638 Qt Terminal: make sure that all Qt objects are only initialized after main()

None
closed-accepted
nobody
None
5
2013-10-28
2013-10-08
Thomas Bleher
No

Qt Terminal: make sure that all Qt objects are only initialized after main()

This is necessary to avoid the Static Initialization Order Fiasco, where
Qt objects are initialized before the Qt library. This problem lead to
crashes when trying to run gnuplot with a Qt terminal on Windows (note
that Windows support for Qt Terminal needs some more patches, but this is
the first critical piece).

The initialization is delayed by wrapping all objects in a struct and
accessing the static struct via an accessor function. This makes sure
that the struct is initialized when the function is called first, not
before.

1 Attachments

Discussion

<< < 1 2 3 (Page 3 of 3)
  • Ethan Merritt
    Ethan Merritt
    2013-10-23

    Re: qt_set_cursor and qt_put_tmptext

    The intent is that the only terminal function that may be called prior to term->init() is term->options(). I see that in practice term->waitforinput() is also called if present. I suppose that's harmless so long as it returns getc(stdin) if mousing is not yet set up. I have edited .../term/README to state this explicitly.

    The pre-initialization calls to term->set_cursor and term->put_tmptext are a bug. I have now fixed that in cvs for both 4.6 and 4.7

    Thanks for finding this.

    Re: gp_exit(). There is a Feature Request that gnuplot be consistent about returning status to the shell when it exits. That isn't practical right now, but if we do go to the trouble of introducing a single exit path it might become so.

     
  • Ethan Merritt
    Ethan Merritt
    2013-10-25

    This is looking pretty good. I've given it a workout under qt4 and qt5 on both 32-bit and 64-bit builds and clang/llvm as well as gcc. All my testing used linux, though, so I can't say whether there are wrinkles that will show up elsewhere. I have access to OpenVMS and Sun machines, but they don't have qt installed (if there even is such a port) so there's not much I can test.

    I'm going to break this out into 2 or 3 stages.

    Stage 1 will be to modify the core code and outboard terminal drivers to use the private gp_exit() and gp_atexit() routines. I'll add this to CVS soon.
    I am not happy extending this to the larger set of [antique? legacy? whatever you want to call them] drivers like next and openstep. I am dubious that anyone is using these, and I figure any changes to the code is as likely to break something as fix something.

    The change to add a constructor to qt_term.cpp will be stage 2. It's looking good but I still see 1 memory leak in valgrind traces. It would be nice to either squash that or document that it is unavoidable.

    I haven't looked at the 2 more recent patches yet.

     
  • Ethan Merritt
    Ethan Merritt
    2013-10-25

    • status: open --> pending-accepted
    • Group: -->
     
  • Thomas Bleher
    Thomas Bleher
    2013-10-25

    I agree that patching old drivers is more risk than it's worth, considering that we cannot test the changes.

    I looked into the Valgrind traces (with --leak-check=full) and see several errors. The most common ends in "QLibraryPrivate::findOrCreate". I don't think we can do anything about this one (short of opening a bug against Qt). As far as I understood the code, Qt loads various libraries dynamically (things like Xrandr and Xcursor), and does not ever unload them.

    The other major offender seems to be glib, when Qt initializes its event loop. I don't know much about glib and its interaction with Qt, so I can't say much about that one. But there seems to be no cleanup function for QEventDispatcherGlibPrivate, which handles access to the glib context, so I suspect that we also cannot do anything about that.

    If you see any other valgrind issues, please post them here - maybe I have overlooked some.

    One think that would help here is not linking gnuplot against QtGui and not using QApplication. Theoretically it would be enough to link gnuplot_qt against QtGui. But then all font handling would have to move to gnuplot_qt and gnuplot would have to query the terminal for the font settings. This is certainly doable, but needs more time than I currently have for this issue.

    Thank you very much for looking at the patches and integrating the first part of it. I greatly appreciate you continuing work on gnuplot!

     
  • Ethan Merritt
    Ethan Merritt
    2013-10-28

    • status: pending-accepted --> closed-accepted
     
<< < 1 2 3 (Page 3 of 3)