Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reloading plugin doesn't disconnect signals properly #9

Closed
chojje opened this issue Dec 12, 2016 · 4 comments
Closed

Reloading plugin doesn't disconnect signals properly #9

chojje opened this issue Dec 12, 2016 · 4 comments
Labels

Comments

@chojje
Copy link

chojje commented Dec 12, 2016

If a method has been connected to a signal from the UI, it will remain connected even though the plugin has been reloaded. This means the signal being fired causes the method to be called, which crashes the plugin because the method being called "exists" but has a NoneType.

I expect this to be fixed by making sure all methods of the reloading plugin are disconnected from the signal.

@borysiasty
Copy link
Owner

Good point, could you please give an example of an affected plugin?

The reloader plugin simply calls QGIS api for unloading the plugin and loading again, so I believe the same problem occurs if you unload the plugin by unchecking it in the plugin manager.

In this case we'd need to scan the plugin code if there are any signals not properly disconnected. The question is, when to perform such test:

  1. on plugin unload, making a workaround for defective plugins
  2. on plugin load, rejecting a defective plugin
  3. when accepting the plugin to the repository

I believe 1. is not an option. The second option shows the problem to the author very early, what is good, but it slows down QGIS start. The third option requires deploying a testing infrastructure in the repository, but I believe it's the best solution.

We should raise a discussion on the qgis-developer mailing list. Unfortunately I'm not able to focus on this problem before late January, so feel free to go on and move it to the list. Otherwise I'll do it in two months.

@chojje
Copy link
Author

chojje commented Dec 12, 2016

@borysiasty This happened when debugging a custom component. I'll see if I can give you a concrete example.

@AlisterH
Copy link

This seems like it could be related, but if not, let me know and I can file it as a separate ticket (either for QGIS or the plugin reloader - whichever you think is the culprit):

I have seen many plugins that properly unload and reload when I disable and then reenable them in the plugin manager, but are not properly unloaded when using the plugin reloader.

An example is the OSM place search plugin, which is listed as 'nominatim' in the plugin reloader. Reload it with the plugin reloader, then right click on the toolbar and you'll see that it is now listed twice in the list of panels - only one is enabled, but you can enable and disable the panels separately, so there seem to be two instances.
For some reason I thought that reloading it again didn't result in a third instance, but I've just tested reloading it repeatedly and have ended up with six instances.
Disable and then reenable the OSM place search in the plugin manager and there is no such problem.
I have seen the same behaviour with plugin toolbars i.e. reload a plugin and get a second copy of its toolbar.

Perhaps there is something wrong with the OSM place search plugin and all others that do this, but if that's the case, why does disabling and then disabling in the plugin manager work correctly?

The reloader plugin simply calls QGIS api for unloading the plugin and loading again, so I believe the same problem occurs if you unload the plugin by unchecking it in the plugin manager.

But since behaviour with the plugin reloader is different from disabling and then enabling in the plugin manager, is there something missing in the plugin reloader? Or does the plugin manager do something that isn't exposed in the Python API?

@borysiasty
Copy link
Owner

Hi @AlisterH!

I opened a ticket for the OSM Place Search: xcaeag/Nominatim-Qgis-Plugin#19

When you disable the plugin in the manager, its dock widget remains working, so once you tick it on again, I doubt if it's effectively reloaded at all. The difference between reloader and manager is because the manager just call unload, while the reloader is made to be more stubborn and try to do its best. As a result, you see the old abandoned instance(s) and the new freshly loaded.

If you know more such plugins, please let me know. I'll see if there is something to improve in the installer or to report to their authors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants