Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

On segunda-feira, 17 de outubro de 2016 14:46:29 PDT Lars Knoll wrote:
> I have to agree with Thiago. For QFactoryLoader, it doesn't make a lot of
> sense to try to unload the plugins, as we only do that on program exit.
> 
> It would be great to make plugins safe to unload, but we all know that this
> is tricky to get right. In addition, we don't even control all the plugins
> we might be loading (thing about additional image formats of styles
> provided by others), so we have no way of knowing whether we can safely
> unload the plugin.
> 
> So I think it's the right thing to change this in 5.6 and not unload those
> plugins on app exit.

https://codereview.qt-project.org/174911

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Lars Knoll]

On 17 Oct 2016, at 15:27, Thiago Macieira 
> wrote:

Em segunda-feira, 17 de outubro de 2016, às 15:50:04 PDT, Konstantin Tokarev
escreveu:
17.10.2016, 15:48, "Thiago Macieira" 
>:
Em segunda-feira, 17 de outubro de 2016, às 13:32:04 PDT, Giuseppe
D'Angelo

escreveu:
Il 14/10/2016 19:44, Thiago Macieira ha scritto:
We are talking in this change about QFactoryLoader, which is a Qt
API
and
which is ALWAYS used until program exit.

I meant internal Qt API.

... doesn't this sound like a declaration that we're not able to have
internal Qt APIs for Qt plugins that are good enough for having the
plugins clean up after themselves, and be able to be unloaded without
crashing?

Correct.

Sure, it's each plugin's fault -- especially the NM bearer plugin -- but I
don't have time to figure out everything that's wrong with it. I'd rather
apply this hammer to the problem than leave applications crashing on
exit.>
Anyhow, I've outlined a plan in my other email.

It may make sense to add generic API for plugin to report if it can be
safely dlclosed(), by default everything is unsafe

Please read again the email that Peppe replied to:

This is about QFactoryLoader, which is loads plugins at application starts and
NEVER attempts to close them anyway. The only time we'd try to unload is at
application exit, which is exactly when it doesn't matter because we're
exiting anyway.

I have to agree with Thiago. For QFactoryLoader, it doesn't make a lot of sense 
to try to unload the plugins, as we only do that on program exit.

It would be great to make plugins safe to unload, but we all know that this is 
tricky to get right. In addition, we don't even control all the plugins we 
might be loading (thing about additional image formats of styles provided by 
others), so we have no way of knowing whether we can safely unload the plugin.

So I think it's the right thing to change this in 5.6 and not unload those 
plugins on app exit.

Cheers,
Lars

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em segunda-feira, 17 de outubro de 2016, às 15:50:04 PDT, Konstantin Tokarev 
escreveu:
> 17.10.2016, 15:48, "Thiago Macieira" :
> > Em segunda-feira, 17 de outubro de 2016, às 13:32:04 PDT, Giuseppe
> > D'Angelo
> > 
> > escreveu:
> >>  Il 14/10/2016 19:44, Thiago Macieira ha scritto:
> >>  >> > We are talking in this change about QFactoryLoader, which is a Qt
> >> API
> >>  >> > and
> >>  >> > which is ALWAYS used until program exit.
> >>  >
> >>  > I meant internal Qt API.
> >> 
> >>  ... doesn't this sound like a declaration that we're not able to have
> >>  internal Qt APIs for Qt plugins that are good enough for having the
> >>  plugins clean up after themselves, and be able to be unloaded without
> >>  crashing?
> > 
> > Correct.
> > 
> > Sure, it's each plugin's fault -- especially the NM bearer plugin -- but I
> > don't have time to figure out everything that's wrong with it. I'd rather
> > apply this hammer to the problem than leave applications crashing on
> > exit.> 
> >>  Anyhow, I've outlined a plan in my other email.
> 
> It may make sense to add generic API for plugin to report if it can be
> safely dlclosed(), by default everything is unsafe

Please read again the email that Peppe replied to:

This is about QFactoryLoader, which is loads plugins at application starts and 
NEVER attempts to close them anyway. The only time we'd try to unload is at 
application exit, which is exactly when it doesn't matter because we're 
exiting anyway.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Konstantin Tokarev]

17.10.2016, 15:48, "Thiago Macieira" :
> Em segunda-feira, 17 de outubro de 2016, às 13:32:04 PDT, Giuseppe D'Angelo
> escreveu:
>>  Il 14/10/2016 19:44, Thiago Macieira ha scritto:
>>  >> > We are talking in this change about QFactoryLoader, which is a Qt API
>>  >> > and
>>  >> > which is ALWAYS used until program exit.
>>  >
>>  > I meant internal Qt API.
>>
>>  ... doesn't this sound like a declaration that we're not able to have
>>  internal Qt APIs for Qt plugins that are good enough for having the
>>  plugins clean up after themselves, and be able to be unloaded without
>>  crashing?
>
> Correct.
>
> Sure, it's each plugin's fault -- especially the NM bearer plugin -- but I
> don't have time to figure out everything that's wrong with it. I'd rather 
> apply
> this hammer to the problem than leave applications crashing on exit.
>
>>  Anyhow, I've outlined a plan in my other email.

It may make sense to add generic API for plugin to report if it can be safely 
dlclosed(), by default everything is unsafe

-- 
Regards,
Konstantin
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em segunda-feira, 17 de outubro de 2016, às 13:32:04 PDT, Giuseppe D'Angelo 
escreveu:
> Il 14/10/2016 19:44, Thiago Macieira ha scritto:
> >> > We are talking in this change about QFactoryLoader, which is a Qt API
> >> > and
> >> > which is ALWAYS used until program exit.
> > 
> > I meant internal Qt API.
> 
> ... doesn't this sound like a declaration that we're not able to have
> internal Qt APIs for Qt plugins that are good enough for having the
> plugins clean up after themselves, and be able to be unloaded without
> crashing?

Correct.

Sure, it's each plugin's fault -- especially the NM bearer plugin -- but I 
don't have time to figure out everything that's wrong with it. I'd rather apply 
this hammer to the problem than leave applications crashing on exit.

> Anyhow, I've outlined a plan in my other email.


-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em segunda-feira, 17 de outubro de 2016, às 11:23:29 PDT, Marc Mutz escreveu:
> Please don't equate 'resource' with 'memory'. Memory is just one of many
> resources a program may have acquired, and the _only_ resource which does
> not need to be released at program exit.

Well, no. It does not need to close file descriptors either, they are closed by 
the OS. In fact, most OS resources are automatically freed when the 
application exits, though on Unix systems that's usually associated with file 
descriptors, such as file locks.

It may have to do the proper shutdown protocol on them, be it save the proper 
data, send the proper disconnection information on a socket or remove 
temporary files. But there are a couple of file descriptors applications seldom 
ever close, like stdin, stdout and stderr.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Giuseppe D'Angelo]

Il 14/10/2016 19:44, Thiago Macieira ha scritto:
>> > We are talking in this change about QFactoryLoader, which is a Qt API and
>> > which is ALWAYS used until program exit.
>  
> I meant internal Qt API.

... doesn't this sound like a declaration that we're not able to have
internal Qt APIs for Qt plugins that are good enough for having the
plugins clean up after themselves, and be able to be unloaded without
crashing?

Anyhow, I've outlined a plan in my other email.

Cheers,
-- 
Giuseppe D'Angelo | giuseppe.dang...@kdab.com | Senior Software Engineer
KDAB (UK) Ltd., a KDAB Group company | Tel: UK +44-1625-809908
KDAB - Qt, C++ and OpenGL Experts



smime.p7s
Description: Firma crittografica S/MIME
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Edward Welbourne]

Marc Mutz
> Please don't equate 'resource' with 'memory'. Memory is just one of many
> resources a program may have acquired, and the _only_ resource which does not
> need to be released at program exit.

Open file descriptors ?
There may be others, but that's the one I tend to think of.
... and I did qualify with "that gets released on exit" language.

Eddy.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Marc Mutz]

On Monday 17 October 2016 11:12:23 Edward Welbourne wrote:
> Liang Jian said:
> >I would not to say "still reachable" simply means the allocation is
> >
> > not leaked.
> 
> Whether you call it a leak or not is just a matter of labelling; what
> matters is resource use.  Does it increase without limit ?  If it
> doesn't, I'm not too fussed whether you call it a leak: it would, of
> course, be nice to use less of any resource, but a bounded amount of
> one-off resource use (that exit() releases) isn't a big deal (unless
> it's a very big allocation).

Please don't equate 'resource' with 'memory'. Memory is just one of many 
resources a program may have acquired, and the _only_ resource which does not 
need to be released at program exit.

Thanks,
Marc

-- 
Marc Mutz  | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt, C++ and OpenGL Experts
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Edward Welbourne]

Liang Jian said:
>I would not to say "still reachable" simply means the allocation is
> not leaked.

Whether you call it a leak or not is just a matter of labelling; what
matters is resource use.  Does it increase without limit ?  If it
doesn't, I'm not too fussed whether you call it a leak: it would, of
course, be nice to use less of any resource, but a bounded amount of
one-off resource use (that exit() releases) isn't a big deal (unless
it's a very big allocation).

>Think about the following senario: A programer allocate a big chunk
> of memory from heap and hold it in a global variable, supposed that by
> design this memory chunk will not used any more at some point and is
> aimed to be released to save memory, but the programmer forget to free
> it, this memory will be considered "still reachable" on exit but the
> fact is that this behaviour do harm to the program since it will waste
> more memory when running the program. This allocation should be
> considered as leak.

It is, of course, always desirable to release memory once one can:
however, this is not so desirable as to make it worth crashing to catch
every last byte.

>Maybe the "still reachable" memory allocations caused by not
> destructing global variables of qt plugin will not do any harm to qt
> app, but others may do harm, how can we easily distinguish the bad
> ones from the good ones? I think the best approach is to make sure we
> release all memory allocated from heap on exit.

The only value of doing deallocations on exit is to make it possible to
catch leaks; and good leak-detection tools distinguish between
still-referenced resources and resources the program has allocated and
forgotten the details it would need in order to release.  In any case,
as long as a given still-held-on-exit resource shall be released by exit
(and there's only one of it), one can record this in prior runs and
ignore it in the tool's reports for later runs.  ISTR valgrind can even
be configured to filter out expected leaks, so that the ignoring is
automatic.

While it is good hygiene to make sure one *has* kept track of the means
to release all resources allocated - and one can most robustly be sure
of this by actually releasing them - it is not really necessary to
release all resources, when exit takes care of doing that for you.  The
problem isn't that we're holding onto resources on exit; it's that we're
using more resources than we should; and this - as Thiago points out -
isn't really a big deal unless there's a way for it to cycle, adding
ever more allocations that won't be released.

>BTW, not every memory leak detector can tell you which memory is
> "still reachable", I used bulitin memory leak detector of debug
> version of msvcrt, inspector from Intel and C++ memory leak detector
> from softwareverify.com, they don't have
> the concept of "still reachable".

Then they're failing at an important part of what their tool should be
doing.  Encourage them to do better, or switch to Valgrind (which is
free, as in liberty, and not just gratis for the first thirty days).

>Anyway, I still consider that we should introduce a method to
> disable the not unloading qt plugin feature at runtime, maybe we can
> only disable this feature in a developer build of qt and that will not
> cause any crash in a release build.

Introducing a method to dlclose() won't save you from leaks on exit;
it'll just make exit happen at some random earlier time, before the user
asked for it, when some attempt is made to (release or) exercise a
resource created by the plugin, on finding that the method to do that
(or some static data it needs to reference while doing it) has been
unloaded.  So the resource can't be released - you can crash trying, or
not try.

It *is* frustrating: plugin architectures where you can load and unload
do make it possible to upgrade plugins on the fly, which is a very cool
thing to do in a long-running application.  (I've worked with the code
to do this in a web server, for example - we had a process that aimed
for (and delivered) uptimes measured in years; getting it to handle DST
transitions in its log files was one of the fun riddles I got to solve -
so being able to use later releases of a plugin was actually quite
important.)  The design constraints that make that possible, however,
tend to limit one's freedom to do creative O-O things with the plugins
(that web server was written in C); maybe it'd be possible to design a
better plugin architecture, but we're stuck with one that can't unload
safely.  Too many things it's reasonable for a plugin author to do (in
C++) are apt to make unloading fatal - and we have lots of crash bugs to
prove it,

Eddy.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em domingo, 16 de outubro de 2016, às 17:51:45 PDT, Liang Jian escreveu:
> I would not to say "still reachable" simply means the allocation is not
> leaked.

Doesn't matter. I won't fix a leak something that happened *only* because the 
application exited.

For me, a legitimate leak requires that the operation be repeatable and cause 
increased memory usage on each repeat. If you can't repaet the operation, then 
I won't fix it.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Liang Jian]

I would not to say "still reachable" simply means the allocation is not
leaked.
Think about the following senario: A programer allocate a big chunk of
memory from heap and hold it in a global variable, supposed that by design
this memory chunk will not used any more at some point and is aimed to be
released to save memory, but the programmer forget to free it, this memory
will be considered "still reachable" on exit but the fact is that this
behaviour do harm to the program since it will waste more memory when
running the program. This allocation should be considered as leak.
Maybe the "still reachable" memory allocations caused by not
destructing global variables of qt plugin will not do any harm to qt app,
but others may do harm, how can we easily distinguish the bad ones from the
good ones? I think the best approach is to make sure we release all memory
allocated from heap on exit.
BTW, not every memory leak detector can tell you which memory is "still
reachable", I used bulitin memory leak detector of debug version of msvcrt,
inspector from Intel and C++ memory leak detector from softwareverify.com,
they don't have the concept of "still reachable".
Anyway, I still consider that we should introduce a method to disable
the not unloading qt plugin feature at runtime, maybe we can only disable
this feature in a developer build of qt and that will not cause any crash
in a release build.


On Sat, Oct 15, 2016 at 11:48 PM, Thiago Macieira  wrote:

> Em sábado, 15 de outubro de 2016, às 15:33:53 PDT, Liang Jian escreveu:
> > But I am still curious about that If we don't unload the plugin, will
> > the destructor of the gobal object in it be called? If it is not called,
> > what if the gobal object of the plugin hold some memory allocated from
> heap?
>
> It's easy to test.
>
> In any case, I don't consider leaks those that happen because the memory
> wasn't freed at program exit while holding a pointer to it. In fact,
> neither
> does valgrind: those are "still reachable" allocations.
>
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
>   Software Architect - Intel Open Source Technology Center
>
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em sábado, 15 de outubro de 2016, às 15:33:53 PDT, Liang Jian escreveu:
> But I am still curious about that If we don't unload the plugin, will
> the destructor of the gobal object in it be called? If it is not called,
> what if the gobal object of the plugin hold some memory allocated from heap?

It's easy to test.

In any case, I don't consider leaks those that happen because the memory 
wasn't freed at program exit while holding a pointer to it. In fact, neither 
does valgrind: those are "still reachable" allocations.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Liang Jian]

Yes, you are right. After applying the patch from
https://codereview.qt-project.org/#/c/140750/ I didn't notice any leak
report under Windows.
Sorry for the noise.

But I am still curious about that If we don't unload the plugin, will
the destructor of the gobal object in it be called? If it is not called,
what if the gobal object of the plugin hold some memory allocated from heap?

On Sat, Oct 15, 2016 at 2:07 AM, Thiago Macieira 
wrote:

> Em sexta-feira, 14 de outubro de 2016, às 16:08:27 PDT, Liang Jian
> escreveu:
> > I don't care custom plugin, What I care is the plugins loaded by qt
> > itself, such as qpa plugin, images format plugins ..., These plugins are
> > needed for all qt programs, we can't avoid the leaks caused by not
> > unloading these plugins if my understanding to this issue is right (Maybe
> > you don't think that are leaks but they will confuse memory leak
> detector),
> > that is why I hope to introduce a method to disable this feature at
> runtime.
>
> What do you prefer?
>
> 1) leak
> 2) crash and leak
>
> Either way, the dlclose() call will not happen.
>
> Note that this has nothing to do with freeing memory. Only the dlclose().
>
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
>   Software Architect - Intel Open Source Technology Center
>
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em sexta-feira, 14 de outubro de 2016, às 11:07:34 PDT, Thiago Macieira 
escreveu:
> What do you prefer?
> 
> 1) leak
> 2) crash and leak
> 
> Either way, the dlclose() call will not happen.
> 
> Note that this has nothing to do with freeing memory. Only the dlclose().

Actually, there's no distinction: both are leaks when the process exits.

The only difference is the cause of the exit: it's either a clean exit by 
returning from main, or a crash.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em sexta-feira, 14 de outubro de 2016, às 16:08:27 PDT, Liang Jian escreveu:
> I don't care custom plugin, What I care is the plugins loaded by qt
> itself, such as qpa plugin, images format plugins ..., These plugins are
> needed for all qt programs, we can't avoid the leaks caused by not
> unloading these plugins if my understanding to this issue is right (Maybe
> you don't think that are leaks but they will confuse memory leak detector),
> that is why I hope to introduce a method to disable this feature at runtime.

What do you prefer?

1) leak
2) crash and leak

Either way, the dlclose() call will not happen.

Note that this has nothing to do with freeing memory. Only the dlclose().

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em sexta-feira, 14 de outubro de 2016, às 07:10:00 PDT, Thiago Macieira 
escreveu:
> We are talking in this change about QFactoryLoader, which is a Qt API and
> which is ALWAYS used until program exit.
 
I meant internal Qt API.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Liang Jian]

I don't care custom plugin, What I care is the plugins loaded by qt
itself, such as qpa plugin, images format plugins ..., These plugins are
needed for all qt programs, we can't avoid the leaks caused by not
unloading these plugins if my understanding to this issue is right (Maybe
you don't think that are leaks but they will confuse memory leak detector),
that is why I hope to introduce a method to disable this feature at runtime.

On Fri, Oct 14, 2016 at 1:10 PM, Thiago Macieira 
wrote:

> Em sexta-feira, 14 de outubro de 2016, às 09:11:48 CEST, Liang Jian
> escreveu:
> > It is a pity that qt develpers have made the dicision of not
> unloading
> > plugins and I have to accept the reality.
> > But I think we should at least  introduce a method to disable this
> > feature at runtime (such as through a enviroment variable) ?
>
> Use QLibrary for your own plugins.
>
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
>   Software Architect - Intel Open Source Technology Center
>
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em sexta-feira, 14 de outubro de 2016, às 09:11:48 CEST, Liang Jian escreveu:
> It is a pity that qt develpers have made the dicision of not unloading
> plugins and I have to accept the reality.
> But I think we should at least  introduce a method to disable this
> feature at runtime (such as through a enviroment variable) ?

Use QLibrary for your own plugins.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em quinta-feira, 13 de outubro de 2016, às 20:46:46 CEST, André Pönitz 
escreveu:
> It's not about most that don't, but those that do, or that would like to.
> 
> At a certain level of complexity one doesn't only want to load plugins
> on demand, but also unload them when not needed use anymore, or even
> unload/reload as part of an update.

We can revisit the idea for QPluginLoader, which is a public API.

We are talking in this change about QFactoryLoader, which is a Qt API and 
which is ALWAYS used until program exit.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Liang Jian]

It is a pity that qt develpers have made the dicision of not unloading
plugins and I have to accept the reality.
But I think we should at least  introduce a method to disable this
feature at runtime (such as through a enviroment variable) ?

On Thu, Oct 13, 2016 at 11:16 PM, Thiago Macieira  wrote:

> Em quinta-feira, 13 de outubro de 2016, às 22:00:35 CEST, Liang Jian
> escreveu:
> > Not unloading plugin is really a bad idea.
> > That will make any memory leak detector report tons of leaks even
> run a
> > simplest qt widgets program. Find and fix 'real' memory leak will be much
> > harder than before.
>
> The decision is already taken: Qt has stopped unloading plugins in Qt 5.7.
>
> The question here is what we should do in 5.6.
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
>   Software Architect - Intel Open Source Technology Center
>
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[André Pönitz]

On Thu, Oct 13, 2016 at 08:16:50PM +0200, Thiago Macieira wrote:
> Em quinta-feira, 13 de outubro de 2016, às 19:47:57 CEST, André Pönitz 
> escreveu:
> > I am not concerned about toy applications that gets started and closed
> > by the minute.
> 
> Right. Most applications don't unload their plugins, except at program exit.

It's not about most that don't, but those that do, or that would like to.

At a certain level of complexity one doesn't only want to load plugins
on demand, but also unload them when not needed use anymore, or even
unload/reload as part of an update.

Andre'
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em quinta-feira, 13 de outubro de 2016, às 19:47:57 CEST, André Pönitz 
escreveu:
> I am not concerned about toy applications that gets started and closed
> by the minute.

Right. Most applications don't unload their plugins, except at program exit.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Konstantin Tokarev]

13.10.2016, 20:39, "André Pönitz" :
> On Thu, Oct 13, 2016 at 08:01:57PM +0300, Konstantin Tokarev wrote:
>>  > I still consider the approach of not unloading plugins fundamentally
>>  > wrong. It only deepens the trench between Qt and valid approaches at
>>  > software architectures.
>>
>>  I think not unloading plugins is fundamentally right thing to do, if
>>  user does not request unload explicitly.
>
> What should happen *if* the user requests unloading explictly?

I think it should be unloaded completely if platform supports that.

>
> Andre'
>
> PS:
>
>>  * It's not supported on all platforms (e.g., uclibc and musl implement
>>  dlclose() as a stub).
>
> Qt was never about providing common subsets of all supported platforms.
>
>>  * At application exit OS will unload plugins and clean up related
>>  resources much faster than it could be implemented with unload in
>>  destructors.
>
> I am not concerned about toy applications that gets started and closed
> by the minute.

* Do your applications load different plugins every minute? In many cases same
plugins are needed for the whole lifetime of application.

* In case of very large applications shutdown time is important as well,
as it can become annoyingly large.

-- 
Regards,
Konstantin
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[André Pönitz]

On Thu, Oct 13, 2016 at 08:01:57PM +0300, Konstantin Tokarev wrote:
> > I still consider the approach of not unloading plugins fundamentally
> > wrong. It only deepens the trench between Qt and valid approaches at
> > software architectures.
> 
> I think not unloading plugins is fundamentally right thing to do, if
> user does not request unload explicitly.

What should happen *if* the user requests unloading explictly?

Andre'

PS:

> * It's not supported on all platforms (e.g., uclibc and musl implement
> dlclose() as a stub).

Qt was never about providing common subsets of all supported platforms.

> * At application exit OS will unload plugins and clean up related
> resources much faster than it could be implemented with unload in
> destructors.

I am not concerned about toy applications that gets started and closed
by the minute.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Konstantin Tokarev]

13.10.2016, 19:39, "André Pönitz" :
> On Thu, Oct 13, 2016 at 03:30:22PM +0100, Sergio Martins wrote:
>>  On 2016-10-12 20:59, Thiago Macieira wrote:
>>  >Hello
>>  >
>>  >We've got a number of issues that got fixed in 5.7 by the change that made
>>  >QFactoryLoader stop unloading plugins (notably, the Network Manager bearer
>>  >plugin). Given that QtDBus in 5.6 is now heavily threaded, a number of new
>>  >issues have cropped up. I've managed to fix some, but reports from Ubuntu
>>  >developers indicate the latest fix only uncovered the next issue.
>>
>>  Hi,
>>
>>  More importantly, this also solves all the QStringLiteral related crashes
>>  we've been seeing.
>
> These would be fixable by going back to QLatin1String.
>
>>  We need this in 5.6, but it's too big a behaviour change for an LTS release.
>>  We need at least a variable to turn it on/off.
>>
>>  Suggestion which me and peppe discussed:
>>
>>  Make it opt-in in 5.6.3 and write in the ChangeLog that there will be a
>>  behaviour change in 5.6.4.
>>  Make it opt-out in 5.6.4 (bringing it on-par with 5.8).
>>
>>  This gives the user a pre-warning of 3 months, it can still be turned off
>>  and fixes many crashes.
>
> I still consider the approach of not unloading plugins fundamentally
> wrong. It only deepens the trench between Qt and valid approaches
> at software architectures.

I think not unloading plugins is fundamentally right thing to do, if user does 
not
request unload explicitly.

Rationale:

* It's risky when plugin uses any non-trivial piece of software, leaving alone 
any
Qt-specific issues, so it's a responsibility of user to ensure that plugin is 
doing
it's deinitialization correctly on all target platforms, and only allow true 
unloading
if this is the case.

* It's not supported on all platforms (e.g., uclibc and musl implement dlclose()
as a stub).

* At application exit OS will unload plugins and clean up related resources much
faster than it could be implemented with unload in destructors.

-- 
Regards,
Konstantin
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[André Pönitz]

On Thu, Oct 13, 2016 at 03:30:22PM +0100, Sergio Martins wrote:
> On 2016-10-12 20:59, Thiago Macieira wrote:
> >Hello
> >
> >We've got a number of issues that got fixed in 5.7 by the change that made
> >QFactoryLoader stop unloading plugins (notably, the Network Manager bearer
> >plugin). Given that QtDBus in 5.6 is now heavily threaded, a number of new
> >issues have cropped up. I've managed to fix some, but reports from Ubuntu
> >developers indicate the latest fix only uncovered the next issue.
> 
> Hi,
> 
> 
> More importantly, this also solves all the QStringLiteral related crashes
> we've been seeing.

These would be fixable by going back to QLatin1String.

> We need this in 5.6, but it's too big a behaviour change for an LTS release.
> We need at least a variable to turn it on/off.
> 
> 
> Suggestion which me and peppe discussed:
> 
> Make it opt-in in 5.6.3 and write in the ChangeLog that there will be a
> behaviour change in 5.6.4.
> Make it opt-out in 5.6.4 (bringing it on-par with 5.8).
> 
> This gives the user a pre-warning of 3 months, it can still be turned off
> and fixes many crashes.

I still consider the approach of not unloading plugins fundamentally
wrong. It only deepens the trench between Qt and valid approaches
at software architectures.

Andre'
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em quinta-feira, 13 de outubro de 2016, às 22:00:35 CEST, Liang Jian escreveu:
> Not unloading plugin is really a bad idea.
> That will make any memory leak detector report tons of leaks even run a
> simplest qt widgets program. Find and fix 'real' memory leak will be much
> harder than before.

The decision is already taken: Qt has stopped unloading plugins in Qt 5.7.

The question here is what we should do in 5.6.
-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Giuseppe D'Angelo]

On 12/10/16 20:59, Thiago Macieira wrote:

We've got a number of issues that got fixed in 5.7 by the change that made
QFactoryLoader stop unloading plugins


It's actually 5.8, isn't it? 494376f980e96339b6f1eff7c41336ca4d853065 is 
in 5.8 (and has a documentation bug as it states 5.7!).


Which opens the question of what to do in 5.7, and IMHO the same that 
should happen in 5.6 (see Sérgio's mail) -- 5.7.2 opt-in, 5.7.3 (if it 
ever happens) opt-out.


Cheers,
--
Giuseppe D'Angelo | giuseppe.dang...@kdab.com | Senior Software Engineer
KDAB (UK) Ltd., a KDAB Group company | Tel: UK +44-1625-809908
KDAB - The Qt, C++ and OpenGL Experts



smime.p7s
Description: S/MIME Cryptographic Signature
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Sergio Martins]

On 2016-10-12 20:59, Thiago Macieira wrote:

Hello

We've got a number of issues that got fixed in 5.7 by the change that 
made
QFactoryLoader stop unloading plugins (notably, the Network Manager 
bearer
plugin). Given that QtDBus in 5.6 is now heavily threaded, a number of 
new
issues have cropped up. I've managed to fix some, but reports from 
Ubuntu

developers indicate the latest fix only uncovered the next issue.


Hi,


More importantly, this also solves all the QStringLiteral related 
crashes we've been seeing.
We need this in 5.6, but it's too big a behaviour change for an LTS 
release. We need at least a variable to turn it on/off.



Suggestion which me and peppe discussed:

Make it opt-in in 5.6.3 and write in the ChangeLog that there will be a 
behaviour change in 5.6.4.

Make it opt-out in 5.6.4 (bringing it on-par with 5.8).

This gives the user a pre-warning of 3 months, it can still be turned 
off and fixes many crashes.





Regards,
--
Sérgio Martins | sergio.mart...@kdab.com | Senior Software Engineer
Klarälvdalens Datakonsult AB, a KDAB Group company
Tel: Sweden (HQ) +46-563-540090, USA +1-866-777-KDAB(5322)
KDAB - The Qt, C++ and OpenGL Experts
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Liang Jian]

Not unloading plugin is really a bad idea.
That will make any memory leak detector report tons of leaks even run a
simplest qt widgets program. Find and fix 'real' memory leak will be much
harder than before.

On Thursday, October 13, 2016, Thiago Macieira 
wrote:

> Em quarta-feira, 12 de outubro de 2016, às 22:28:33 CEST, André Pönitz
> escreveu:
> > On Wed, Oct 12, 2016 at 09:59:06PM +0200, Thiago Macieira wrote:
> > > Hello
> > >
> > > We've got a number of issues that got fixed in 5.7 by the change that
> made
> > > QFactoryLoader stop unloading plugins (notably,
> >
> > Not unloading plugins is not a fix.
>
> No, it's a workaround. But I'm at a loss to fix the current issue.
>
> The issue at hand is that the plugin was unloaded between a slot being
> invoked
> in a queued connection and the object that would receive the event getting
> deleted. When deleting the object, the events queued to it are deleted.
> When
> metacall events are deleted, the parameters stored in it are destroyed. In
> order for QMetaType to destroy such a type, it needs to call the destroying
> functions registered with it.
>
> Since we do allow breaking of ODR because of hidden visibility / DLLs, the
> functions in question may have belonged to the plugin that was unloaded,
> even
> if the type wasn't exclusive to that plugin.
>
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
>   Software Architect - Intel Open Source Technology Center
>
> ___
> Development mailing list
> Development@qt-project.org 
> http://lists.qt-project.org/mailman/listinfo/development
>
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Em quarta-feira, 12 de outubro de 2016, às 22:28:33 CEST, André Pönitz 
escreveu:
> On Wed, Oct 12, 2016 at 09:59:06PM +0200, Thiago Macieira wrote:
> > Hello
> > 
> > We've got a number of issues that got fixed in 5.7 by the change that made
> > QFactoryLoader stop unloading plugins (notably,
> 
> Not unloading plugins is not a fix.

No, it's a workaround. But I'm at a loss to fix the current issue.

The issue at hand is that the plugin was unloaded between a slot being invoked 
in a queued connection and the object that would receive the event getting 
deleted. When deleting the object, the events queued to it are deleted. When 
metacall events are deleted, the parameters stored in it are destroyed. In 
order for QMetaType to destroy such a type, it needs to call the destroying 
functions registered with it.

Since we do allow breaking of ODR because of hidden visibility / DLLs, the 
functions in question may have belonged to the plugin that was unloaded, even 
if the type wasn't exclusive to that plugin.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] Backporting the "stop unloading plugins" change to 5.6

[André Pönitz]

On Wed, Oct 12, 2016 at 09:59:06PM +0200, Thiago Macieira wrote:
> Hello
> 
> We've got a number of issues that got fixed in 5.7 by the change that made 
> QFactoryLoader stop unloading plugins (notably,

Not unloading plugins is not a fix.

Andre'
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

[Development] Backporting the "stop unloading plugins" change to 5.6

[Thiago Macieira]

Hello

We've got a number of issues that got fixed in 5.7 by the change that made 
QFactoryLoader stop unloading plugins (notably, the Network Manager bearer 
plugin). Given that QtDBus in 5.6 is now heavily threaded, a number of new 
issues have cropped up. I've managed to fix some, but reports from Ubuntu 
developers indicate the latest fix only uncovered the next issue.

If we do stop unloading, the problem goes away.

So, ok to backport?

See https://codereview.qt-project.org/172014 (original 
https://codereview.qt-project.org/140750)
-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development