On Tue, Jun 23, 2009 at 12:17:23PM -0700, Alan Irwin wrote: > On 2009-06-23 15:48+0100 Andrew Ross wrote: > > > On Fri, Jun 19, 2009 at 10:00:41AM -0700, Alan Irwin wrote: > >> Hi Andrew: > >> > >> I have addressed this post to you as well because the previous commit > >> message that put in a mutex for this device implied you had a thread safety > >> test for the qt device at that time: > >> > >> ******** > >> r9765 | airwin | 2009-03-21 08:39:08 -0700 (Sat, 21 Mar 2009) | 8 lines > >> > >> AWI for Alban Rochel. > >> > >> - Introduce a mutex to make driver thread safe. > >> > >> Andrew noticed that when 2 qtwidgets were running on 2 streams, closing > >> them > >> didn't close the driver properly. This issue should be fixed by this > >> change. > >> ******** > >> > >> Could you apply the same test for this new way of handling the qt mutex? > >> Also, if you could share the details of your test (i.e., what example to > >> run > >> interactively and how to identify when the driver is not closed properly), > >> I > >> should be able to incorporate it into our interactive test suite so that > >> anybody could run the test on any platform. > > > > Alan, > > > > The original test just involved running the multiple stream example 14 with > > two devices, one interactive and one file based. Originally there were > > global variables set which depended on the type of the first initialised > > device. > > I think problems occured if you initialised the file based driver first then > > tried to use an interactive driver for the second stream. I would have to > > check though. > > Do you happen to remember what those run-time problems were and how you > triggered them? Testing of GUI's is tricky because results often depend on > exactly which GUI buttons you pushed in what order. In any case, please > give example 14 a thrashing to see if you see anything questionable now.
The problem was specifically that the qt library was global set up differently for interactive and non-interactive devices. Alban fixed this as I recall and current svn version does not exhibit the problems I saw. Basically it crashed if you ran example 14 with a non-interactive qt devices on the first stream and qtwidget on the second stream. This does not happen now. It doesn't actually work very well since example 14 turns off pausing for the second stream and there is no pausing anyway for a non-interactive device, but this is a general point and not a qt one. Anyway, this issue is I think separate from the latest problems found. > > There is now more urgency to extensive testing of the thread safety for the > qt driver as well as the associated plplotqt library. In the latter case, > qt_example shows some memory management issues concerning the mutex. (There > were some build issues for that installed C++ example, but those have now > [revision 10067] been completely fixed for the new CMake-based build system > for the installed examples.) The run-time issue for qt_example is you cannot > exit the associated GUI anymore using the usual methods (e.g., alt+F4). > Instead, you have to hit control-c. I investigated some more with valgrind > and exposed some severe memory management issues with the mutex when all I > did in the resulting GUI was hit alt+F4 to exit. (I get a similar result if > Dmitri's patch is reverted so the issue is probably Alban's mutex to make qt > thread safe is not completely correct and that error propagated to Dmitri's > implementation of that mutex logic.) > > The initial memory management error for qt_example is > > ==27514== Conditional jump or move depends on uninitialised value(s) > ==27514== at 0x7E6BBE6: QtExtWidget::paintEvent(QPaintEvent*) (qt.cpp:1922) > > That line of code is > > if(!cursorParameters.isTracking || cursorParameters.cursor_x<0) return; > > and presumably the issue is caused by one of those two variables not being > properly initialized. I don't know whether this issue has anything to do > with the later much more severe memory management issues found by valgrind > for the mutex, but it is always a good rule of thumb to deal with the first > obvious valgrind issue encountered so can somebody with more knowledge of > the qt.cpp than me fix those uninitialized variables? > > For the record, that later issue starts with the following valgrind message > > ==27514== Invalid read of size 8 > ==27514== at 0x7A41901: pthread_cond_destroy@@GLIBC_2.3.2 (in > /lib/libpthread-2.7.so) > ==27514== by 0x6F797A0: QMutexPrivate::~QMutexPrivate() > (qmutex_unix.cpp:70) > > Hopefully, that will go away once the uninitialized variable is fixed. > > I also tried -dev qtwidget on example 10, and the first (and only) > valgrind error for the qt.cpp codepath used by that device was > > ==27638== Conditional jump or move depends on uninitialised value(s) > ==27638== at 0x6B859FB: plD_eop_qtwidget(PLStream*) (qt.cpp:1807) > > That line of code is > > while(currentPage==widget->pageNumber && handler.isMasterDevice(widget) > && ! pls->nopause) > > and again we need a fix for one or more of those variables not being > properly initialized. > > Finally, I also tried -dev epsqt on example 10, and no valgrind memory > management issues were encountered for that device so it would be wrong to > conclude qt.cpp was riddled with uninitialized variables. :-) But there are > definitely some there for certain code paths as can be seen above. I'll try and take a look at this, unless someone else beats me to it. Andrew ------------------------------------------------------------------------------ _______________________________________________ Plplot-devel mailing list Plplot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/plplot-devel
