Re: [Plplot-devel] [PATCH] qt driver: use QMutexLocker
On 2009-06-23 12:17-0700 Alan W. Irwin wrote: 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. These uninitialized variables (also seen in a valgrind run on python ./pyqt4_test.py) have now been removed by a patch kindly supplied by Alban (revision 10068). This change makes -dev qtwidget completely valgrind clean (just like -dev epsqt), makes qt_example valgrind clean until an attempt is made to exit the GUI with alt-F4, and removes any reference to PLplot code from the valgrind output for python ./pyqt4_test.py Thanks for this fix, Alban! This fix will make it a lot easier to fix the remaining known qt issues which are as follows: * The QMutex destructor called twice issue that Dmitri discovered. This shows up as lots of valgrind output involving PLplot code and a hang (which you can get out of with ctrl-C) when you attempt to exit the qt_example GUI. * The blank GUI issue for python ./pyqt4_test.py. In this case, there is still extensive valgrind output but none of it involves PLplot code. Furthermore, I am pretty sure it is just python/valgrind noise because almost the same large amount of noise is produced if you attempt to use valgrind to analyze an interactive python session where all you do is immediately exit from the python session. So I now suspect the blank GUI issue is simply some programming issue rather than a more exotic memory management issue or uninitialized variable issue. Alan __ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); PLplot scientific plotting software package (plplot.org); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __ Linux-powered Science __ -- ___ Plplot-devel mailing list Plplot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/plplot-devel
Re: [Plplot-devel] [PATCH] qt driver: use QMutexLocker
On Wed, Jun 24, 2009 at 6:50 PM, Alan W.
Irwinir...@beluga.phys.uvic.ca wrote:
* The QMutex destructor called twice issue that Dmitri discovered. This
shows up as lots of valgrind output involving PLplot code and a hang (which
you can get out of with ctrl-C) when you attempt to exit the qt_example GUI.
After thinking and debugging a bit, I found out:
* if you compile with ENABLE_DYNDRIVERS=OFF, the problem doesn't show up
* with ENABLE_DYNDRIVERS=ON qt_example doesn't compile, the linker
cannot find methods from QtExtWidget as well as
plsetqtdev()/plfreeqtdev(). After I add -lplplotqtd to the line 88 of
examples/c++/Makefile.examples.in, qt_example compiles fine. Am I
doing anything wrong?
Now at startup qt_example loads libplplotqtd.so, which contains all
code from qt.cpp, including the code that is supposed to be loaded
dynamically. At some point it calls plinit(), that calls
plLoadDriver(), that loads qt.so -- in fact, the second copy of the
code! Of course, at shutdown of the application destructors are get
called from each library, but they touch the same global data.
In this light, my proposed solution is just a hack. For a proper
solution, I think it is needed to split qt.cpp in two parts:
* libplpllotqtd.so should contain only classes and functions user
needs to see (plsetqtdev()/plfreeqtdev());
* qt.so should contain only functions that are callbacks for plplot
core. qt.so will need to be linked against libplplotqtd.so.
Dmitri
--
main(i,j){for(i=2;;i++){for(j=2;ji;j++){if(!(i%j)){j=0;break;}}if
(j){printf(%d\n,i);}}} /*Dmitri Gribenko griboz...@gmail.com*/
--
___
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel
Re: [Plplot-devel] [PATCH] qt driver: use QMutexLocker
On 2009-06-24 23:13+0300 Dmitri Gribenko wrote: On Wed, Jun 24, 2009 at 6:50 PM, Alan W. Irwinir...@beluga.phys.uvic.ca wrote: * The QMutex destructor called twice issue that Dmitri discovered. This shows up as lots of valgrind output involving PLplot code and a hang (which you can get out of with ctrl-C) when you attempt to exit the qt_example GUI. After thinking and debugging a bit, I found out: * if you compile with ENABLE_DYNDRIVERS=OFF, the problem doesn't show up * with ENABLE_DYNDRIVERS=ON qt_example doesn't compile, the linker cannot find methods from QtExtWidget as well as plsetqtdev()/plfreeqtdev(). After I add -lplplotqtd to the line 88 of examples/c++/Makefile.examples.in, qt_example compiles fine. Am I doing anything wrong? I should have been clearer. I have been working with the new CMake-based build system for the installed examples to get it to properly build qt_example for the ENABLE_DYNDRIVERS=ON case. So you should stick to that case also (assuming you have downloaded and installed cmake-2.6.4 which is a necessity for the new build system for the installed examples). You use this new build system (after make install for the core build system) by simply running cmake $prefix/share/plplot5.9.4/examples cd c++ make qt_example from an initially empty build tree. N.B. the new CMake-based build system for the installed examples is configured by the core build system and finalized by make install for that core build system so there are no cmake options required in the above cmake command. Once we have sorted everything out for the new CMake-based build system for the installed examples and the ENABLE_DYNDRIVERS=ON case, then I will fix up the ENABLE_DYNDRIVERS=OFF case (by not building libplplotqtd.so at all and by linking qt_example to libplplotd instead) and also tweak the old Makefile+pkg-config approach for building the installed examples for both the ENABLE_DYNDRIVERS=ON and ENABLE_DYNDRIVERS=OFF cases. Therefore, I don't think there will be any necessity of splitting qt.cpp, and the patch you sent (in a later post) is probably fine as is. I will look at that tomorrow. Alan __ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); PLplot scientific plotting software package (plplot.org); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __ Linux-powered Science __ -- ___ Plplot-devel mailing list Plplot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/plplot-devel
Re: [Plplot-devel] [PATCH] qt driver: use QMutexLocker
On Fri, Jun 19, 2009 at 10:00:41AM -0700, Alan Irwin wrote: On 2009-06-19 06:35+0300 Dmitri Gribenko wrote: Use QMutexLocker in Qt driver. RAII is always better than managing resources by hand. Hi Dmitri: This patch has been reviewed and approved by Alban, but not tested by him. My light testing (make test_noninteractive in the installed examples tree) showed no obvious issues. Therefore, I have committed this patch (revision 10055). Thanks! 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. Andrew -- Are you an open source citizen? Join us for the Open Source Bridge conference! Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250. Need another reason to go? 24-hour hacker lounge. Register today! http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org ___ Plplot-devel mailing list Plplot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/plplot-devel
Re: [Plplot-devel] [PATCH] qt driver: use QMutexLocker
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. 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_x0) 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. Alan __ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); PLplot scientific plotting software package (plplot.org); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __ Linux-powered
Re: [Plplot-devel] [PATCH] qt driver: use QMutexLocker
On 2009-06-19 06:35+0300 Dmitri Gribenko wrote: Use QMutexLocker in Qt driver. RAII is always better than managing resources by hand. Hi Dmitri: This patch has been reviewed and approved by Alban, but not tested by him. My light testing (make test_noninteractive in the installed examples tree) showed no obvious issues. Therefore, I have committed this patch (revision 10055). Thanks! 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 __ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); PLplot scientific plotting software package (plplot.org); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __ Linux-powered Science __ -- Crystal Reports - New Free Runtime and 30 Day Trial Check out the new simplified licensing option that enables unlimited royalty-free distribution of the report engine for externally facing server and web deployment. http://p.sf.net/sfu/businessobjects ___ Plplot-devel mailing list Plplot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/plplot-devel
