Re: [Plplot-devel] Change to appCounter logic within qt.cpp

[António R. T. <ar...@gm...>]

On Sat, Dec 29, 2018 at 2:28 AM Alan W. Irwin <Ala...@gm...>
wrote:

> On 2018-12-27 16:30-0000 António Rodrigues Tomé wrote:
>
> > As for  the another important change to allow all qt  drivers to be
> called
> > from within a qt application I think your worries are not justifiable as
> > the function I changed is not called when an extqt is used.
> > my change
> >     if(appCounter == 1 && qApp != NULL) ++appCounter; //This was added to
> > allow qt drivers to be called from within a qt program.
> > is in function initQtApp
> > that is called by
> > plD_init_rasterqt
> > plD_init_svgqt
> > plD_init_epspdfqt
> > plD_init_qtwidget
> > plD_init_memqt
> >
> >
> >
> > is not called by
> >
> > plD_init_extqt
> >
> > as this driver is, as I understand to be called from within a qt
> > application so does not try to open another qApp.
>
> Hi António:
>
> I have changed the subject line to something more specific to this
> different topic.
>
> I have been trying to understand why this change had to be made, and
> here is what I have discovered so far by digging my way through the
> qt.cpp code.
>
> All qt devices need a qApp in order to work and the purpose of
> initQtApp (where I have confirmed your statement above that it is
> called from all plD_init* routines *other than plD_init_extqt* that
> are implemented in drivers/qt.cpp) is to create argv memory and an
> associated qApp if a qApp doesn't exist already (either created by a
> previous call to initQtApp or in external qt code such as your own
> external qt application that uses ordinary (non-extqt) qt devices and
> qt_example (which does not use qt devices other than extqt).  And the
> purpose of closeQtApp (which is called from all the plD_tidy_*
> routines implemented in drivers/qt.cpp) is to delete that qApp and
> associated (argv) memory that was initially created in initQtApp.  And
> the purpose of appCounter is to make sure only one creation and only
> one deletion occurs.
>
> Assuming that analysis is correct, I now understand how your external
> qt application failed before your change because your application had
> created a new (or automatic) instance of qApp already so initQtApp
> called indirectly by your use of an ordinary qt device would increment
> appCounter from 0 to 1 and return (since qApp was already non-NULL)
> while when you were done with that ordinary qt device, PLplot would
> tidy up for that device and call closeQtApp which decremented
> appCounter to 0 and therefore would then proceed to attempt to delete
> the qApp *created by your external qt application* in error.
>
> So your fix for that error was to initialize appCounter to 2 within
> initQtApp (if qApp non-NULL and appCounter was 1) so that incorrect
> closeQtApp deletion never occurs for this situation.
>

Yes you are right is exactly that to avoid creating a qtapplication and
most important to prevent deleting the main qt app.


>
> It also follows from the above analysis why extqt (used by
> qt_example) rightly does not call initQtApp.  And when PLplot tidys up
> extqt, closeQtApp gets called with appCounter equal to its static
> initial value of 0, that routine then decrements that counter to -1,
> and therefore an incorrect cleanup of the qApp from qt_example is not done
> (as
> it should be).
>

I'm puzzled my changes do not in any way affect the extqt case. the ext-qt
also  should never  call
closeQtapp() but in fact it  calls it but it is a flaw in code that does
do not arm because as you said appCounter becomes -1 and nothing is done
besides the mutex instruction that i'm still uneasy with them, otherwise it
would kill the program.


> So my gut feeling is this code and also your modification of it is
> pretty fragile, but it does appear to work (at least according to the
> above mental model of what is going on), and I have nothing better to
> offer.
>

it can be fragile on the sense that the original code may be fragile other
than that fits perfectly  within the actual driver approach.

However, the reason I am concerned with what I perceive as fragility
> in this code is I want to be really careful about creating and
> destroying qApp since qt_example currently has memory management
> issues that make it segfault *sometimes* on exit.  And I presume those
> are due to some screwup in the interaction between specific cleanups
> (possibly the one we are discussing above)
> and automatic ones.  So if you can think of a
> cleaner way (perhaps recording the actual qApp value that is created
> in initQtApp that should be destroyed later) to destroy the qApp
> that is specifically created by initQtApp, that would be great.
>

qApp is a macro, a convenient way to access the application pointer
anywhere from any parts of a qt application
so there should be only one and as no need to identify it.


> Please confirm my above analysis is correct or let me know where I
> went wrong.  If it turns out my analysis is correct, but you still do
> not share my concerns about the robustness of the code in its present
> state (i.e., with your change), then I will add information to your
> commit message based on the above analysis and go ahead and push your
> change.
>
> The code changes only inform the driver that there is already an
QApplication running  before plplot is called so do not start a new one
neither do  kill the qtApplication after the plplotstuf is closed. There is
in fact no additional problems related to the changes.

Cheers and an Happy New Year

P.s
Unfortunately  I was not able to make qt-example seg fault in my system so
could not found out what was the problem.

Alan
> __________________________
> Alan W. Irwin
>
> Programming affiliations with the FreeEOS equation-of-state
> implementation for stellar interiors (freeeos.sf.net); the Time
> Ephemerides project (timeephem.sf.net); PLplot scientific plotting
> software package (plplot.sf.net); 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
> __________________________
>


-- 

António Rodrigues Tomé
Universidade da Beira Interior
Instituto D. Luís (lab associado)
email address:
ar...@gm...
ar...@ub...
http://www.researcherid.com/rid/A-5681-2013