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

[Alan W. I. <Ala...@gm...>]

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.

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).

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.

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.

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.

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
__________________________