Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#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 1 of 3)
  • Ethan Merritt
    Ethan Merritt
    2013-10-08

    I'd like to understand better how one would diagnose or debug the problem this patch tries to fix. Can you provide a pointer to discussion somewhere?

    Meanwhile...

    The patch applies cleanly to current CVS, but when I try to compile the revised source I get the following:

    qtterminal/qt_term.cpp: In function void qt_filled_polygon(int, gpiPoint*):
    qtterminal/qt_term.cpp:720:2: error: qt was not declared in this scope
    qtterminal/qt_term.cpp: In function void qt_image(unsigned int, unsigned int, coordval*, gpiPoint*, t_imagecolor):
    qtterminal/qt_term.cpp:727:2: error: qt was not declared in this scope
    qtterminal/qt_term.cpp: In function void qt_hypertext(int, const char*):
    qtterminal/qt_term.cpp:1138:3: error: qt_out was not declared in this scope
    qtterminal/qt_term.cpp:1138:28: error: qt_codec was not declared in this scope
    qtterminal/qt_term.cpp: In function void qt_boxed_text(unsigned int, unsigned int, int):
    qtterminal/qt_term.cpp:1144:2: error: qt_out was not declared in this scope
    qtterminal/qt_term.cpp: In function void qt_modify_plots(unsigned int):
    qtterminal/qt_term.cpp:1151:3: error: qt_out was not declared in this scope
    qtterminal/qt_term.cpp:1153:3: error: qt_out was not declared in this scope
    qtterminal/qt_term.cpp:1155:3: error: qt_out was not declared in this scope
    make[4]: *** [qt_term.o] Error 1
    make[4]: Leaving directory `/home/merritt/cvs/gnuplot-cvs/src'
    make[3]: *** [all-recursive] Error 1
    make[3]: Leaving directory `/home/merritt/cvs/gnuplot-cvs/src'
    make[2]: *** [all] Error 2
    make[2]: Leaving directory `/home/merritt/cvs/gnuplot-cvs/src'
    make[1]: *** [all-recursive] Error 1
    make[1]: Leaving directory `/home/merritt/cvs/gnuplot-cvs'
    make: *** [all] Error 2
    

    Also, are you working with Qt4 or Qt5? Does it matter? I ask partly because when I build the current source with Qt5 there is a problem (segfault) at the time the program exits. This doesn't happen with Qt4, and so far as I can tell the program operates correctly under Qt5 right up until the time it tries to exit. I have been unable to pin down the cause, but now you have me wondering if this patch is related.

     
    Last edit: Ethan Merritt 2013-10-08
  • Ethan Merritt
    Ethan Merritt
    2013-10-08

    So I sprinkled in a few more declarations of
    QtGnuplotState & qt = getQtGnuplotState();
    to get past the compiler errors.

    Now it runs but spews out messages like the following:
    QThreadStorage: Thread 0x8f35fb8 exited after QThreadStorage 30 destroyed
    QThreadStorage: Thread 0x8f35fb8 exited after QThreadStorage 29 destroyed

    What is this telling me?

     
  • Thomas Bleher
    Thomas Bleher
    2013-10-09

    First off, I'm very sorry for the incomplete patch. I forward-ported the patch from an older CVS version of Gnuplot, checked that the patch applied cleanly, but forgot to test it actually compiled :(

    As for the actual issue the patch tries to solve: The general issue is best described in the C++ FAQ: http://www.parashift.com/c++-faq/static-init-order.html

    Whenever there are static initializations (at the file level) in several translation units, it is undefined which of these run first.

    As a simple example, qapplication.cpp in Qt 4.8.4 contains a static integer variable drag_time, which can be queried by calling QApplication::startDragTime(). This variable is initialized in qapplication.cpp as

    static int drag_time = 500;
    

    However, if there is code in another translation unit which does

    static int my_drag_time = QApplication::startDragTime();
    

    the behaviour is undefined, because the static variable drag_time may not have been initialized yet. Initialization is guaranteed to be completed by the time main() is called. Additionally, function scoped static variables are initialized the first time control flow passes over them. That's why I introduced getQtGnuplotState(), with a static variable containing all the state. As long as getQtGnuplotState() is only called after main() (i.e. not from other static initializations), all should be OK because by that time the Qt library is fully initialized.

    I encountered this issue when trying to port the Qt terminal to Windows. On Linux it seems to work (maybe the libraries are initialized first), but this is in no way guaranteed.

    I still use Qt4 and haven't seen the QThreadStorage error messages you mention. I'm currently upgrading my build system and will see if I find anything.

     
  • Ethan Merritt
    Ethan Merritt
    2013-10-09

    But the Static Initialization Order Fiasco arises when references in one compilation unit depend on static initialization in a different compilation unit. Your patch touches only qt_term.cpp, which is a single compilation unit. So far as I can see, none of the variables being initialized are referenced even indirectly by other files. So what exactly is triggering the Fiasco? Can you pin it down to a specific external reference that runs amok? I really like to understand the problem better before accepting that this is the proper fix.

     
  • Thomas Bleher
    Thomas Bleher
    2013-10-09

    The variables in qt_term.cpp indirectly reference static variables in the Qt library. Consider the following example:

    QTextCodec* qt_codec = QTextCodec::codecForLocale();
    

    This code is currently in qt_term.cpp. qt_codec is initialized before main(). Looking at src/corelib/codecs/qtextcodec.cpp (from Qt 4.8.4), we see that codecForLocale() accesses the static variable localeMapper. At this stage of the program execution, however, localeMapper may have any value, so the function potentially returns an arbitrary invalid pointer, which will lead to crash when the pointer is used later. Note that this problem also encompasses constructor calls, so the following simple line may crash:

    QString qt_optionTitle;
    

    The solution implemented in the patch avoids this by not calling any Qt function (not even constructors) until after main().

     
  • Thomas Bleher
    Thomas Bleher
    2013-10-09

    I investigated the code a bit more and suspect that the two examples cited are actually OK, because they only reference statically initialized POD variables, no statically initialized classes which have constructors. I'm not 100% sure what the standard says here, but I think PODs are OK because they are probably initialized before any constructor runs.

    The QLocalSocket code however looks quite suspicious on Windows. I'll look at it again tomorrow and report my findings.

     
  • Ethan Merritt
    Ethan Merritt
    2013-10-09

    FWIW, your patch cures the segfault-on-exit problem I've been having with Qt5. So that's good. But I'd still like to understand exactly why. Both Qt4 and Qt5 emit the "Thread 0x8f35fb8 exited after QThreadStorage 29 destroyed" messages.

     
  • Thomas Bleher
    Thomas Bleher
    2013-10-10

    Can you set a breakpoint on where this message happens and send me the backtrace? If we see which class uses QThreadStorage here, we might be able to pinpoint the problem.

    In Qt4, this is line 199 of src/corelib/thread/qthreadstorage.cpp, function QThreadStorageData::finish.

     
  • Ethan Merritt
    Ethan Merritt
    2013-10-10

    backtrace is here:

    #0  0xb5b204c0 in QThreadStorageData::finish(void**) () from /lib/libQtCore.so.4
    #1  0xb5c399c8 in QCoreApplicationPrivate::~QCoreApplicationPrivate() ()
       from /lib/libQtCore.so.4
    #2  0xb5edccb3 in QApplicationPrivate::~QApplicationPrivate() () from /lib/libQtGui.so.4
    #3  0xb5edce42 in QApplicationPrivate::~QApplicationPrivate() () from /lib/libQtGui.so.4
    #4  0xb5c4e36a in QObject::~QObject() () from /lib/libQtCore.so.4
    #5  0xb5c39920 in QCoreApplication::~QCoreApplication() () from /lib/libQtCore.so.4
    #6  0xb5ee8114 in QApplication::~QApplication() () from /lib/libQtGui.so.4
    #7  0xb580c7a1 in __run_exit_handlers () from /lib/i686/libc.so.6
    #8  0xb580c82d in exit () from /lib/i686/libc.so.6
    #9  0xb57f5a3d in __libc_start_main () from /lib/i686/libc.so.6
    #10 0x080687b1 in _start () at ../sysdeps/i386/start.S:117
    

    This is triggered inside the qt_atexit routine in qt_term.cpp. I would guess that the issue really arises at some earlier point when some per-thread data structures are released. I suspect that the release is not really premature in terms of program function, but apparently the garbage collection system thinks it is. Do you know how to trap this hypothetical early release?

    The other clue I have is from valgrind. Valgrind has in the past reported a ton of "lost" memory on exit from gnuplot when the qt terminal is active. Your patch reduces this by about 90%. Most of the remaining 10% is flagged as resulting from execution of lines qt_term.cpp:417-418 (reproduced below)

    417     QFontMetrics metrics(QFont(qt.currentFontName, qt.currentFontSize));
    418     term->v_char = qt_oversampling * (metrics.ascent() + metrics.descent());
    419     term->h_char = qt_oversampling * metrics.width("0123456789")/10.;
    
    Sample valgrind trace:
    
    ==31081== 136 (8 direct, 128 indirect) bytes in 1 blocks are definitely lost in loss record 338 of 486
    ==31081==    at 0x4028E2A: operator new(unsigned int) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
    ==31081==    by 0x5C094EB: ??? (in /usr/lib/libQtGui.so.4.8.5)
    ==31081==    by 0x5C0B756: ??? (in /usr/lib/libQtGui.so.4.8.5)
    ==31081==    by 0x5C0BF49: QFontEngineFT::init(QFontEngine::FaceId, bool, QFontEngine::GlyphFormat, QByteArray const&) (in /usr/lib/libQtGui.so.4.8.5)
    ==31081==    by 0x5C051DA: QFontEngineX11FT::QFontEngineX11FT(_FcPattern*, QFontDef const&, int) (in /usr/lib/libQtGui.so.4.8.5)
    ==31081==    by 0x5B43C46: ??? (in /usr/lib/libQtGui.so.4.8.5)
    ==31081==    by 0x5B4DBA0: QFontDatabase::load(QFontPrivate const*, int) (in /usr/lib/libQtGui.so.4.8.5)
    ==31081==    by 0x5B2858C: QFontPrivate::engineForScript(int) const (in /usr/lib/libQtGui.so.4.8.5)
    ==31081==    by 0x5B3D414: QFontMetrics::ascent() const (in /usr/lib/libQtGui.so.4.8.5)
    ==31081==    by 0x8169997: qt_graphics (qt_term.cpp:418)
    ==31081==    by 0x814369A: term_start_plot (term.c:557)
    ==31081==    by 0x80A29E5: do_plot (graphics.c:500)
    ==31081==    by 0x80C5D33: plotrequest (plot2d.c:3104)
    ==31081==    by 0x8078CFF: do_line (command.c:619)
    ==31081==    by 0x8078E98: do_string_and_free (command.c:454)
    ==31081==    by 0x8067A9C: main (plot.c:609)
    ==31081==
    
     
    Last edit: Ethan Merritt 2013-10-10
  • Ethan Merritt
    Ethan Merritt
    2013-10-10

    Correction: The backtrace is from after execution of qt_atexit(). If I bypass qt_exit() the same error messages are printed.

     
1 2 3 > >> (Page 1 of 3)