Fix loading/unloading of GDB plugin
ClosedPublic

Authored by antonanikin on Feb 10 2017, 7:19 AM.

Details

Summary

The patch fixes some wrong moments for GDB plugin loading/unloading. The problems are:

  1. If the plugin was loaded and we do unload + load, then memory leaks will happens (GdbLauncher objects).
  2. If the plugin was loaded and we do unload, then we still will see it's config page in the launcher configuration.
  3. If we do unload + load for some plugin with IExecutePlugin extension, then the GDB plugin will not create new GdbLauncher objects.

The new version fixes all this issues.

After review this approach should be also applied to LLDB plugin.

Test Plan

tested with master branch

Diff Detail

Repository
R32 KDevelop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
antonanikin retitled this revision from to Fix loading/unloading of GDB plugin.
antonanikin updated this object.
antonanikin edited the test plan for this revision. (Show Details)
antonanikin added a reviewer: KDevelop.
antonanikin added a subscriber: kdevelop-devel.
antonanikin updated this object.Feb 10 2017, 7:20 AM
mwolff requested changes to this revision.Feb 10 2017, 8:48 AM
mwolff added a reviewer: mwolff.
mwolff added a subscriber: mwolff.

good in principle, but some style nitpicks

debuggers/gdb/debuggerplugin.cpp
58 ↗(On Diff #11144)

use range-based-for in new code please, and wrap strings in QStringLiteral

64 ↗(On Diff #11144)

indent, such that this line is four spaces deeper than the this in the line above

65 ↗(On Diff #11144)

indent, such that this line aligns with the this

69 ↗(On Diff #11144)

dito indent

75 ↗(On Diff #11144)

dito above

debuggers/gdb/debuggerplugin.h
87 ↗(On Diff #11144)

QHash

QMap is sorted, but pointers are inherently random in their value (i.e. the address), so you definitely don't want to sort them.

This revision now requires changes to proceed.Feb 10 2017, 8:48 AM
antonanikin edited edge metadata.
  • Fix inline comments
antonanikin marked 5 inline comments as done.Feb 10 2017, 9:23 AM
antonanikin marked an inline comment as done.
mwolff requested changes to this revision.Feb 12 2017, 11:47 AM
mwolff edited edge metadata.

two more style nitpicks, and one suggestion. otherwise lgtm

debuggers/gdb/debuggerplugin.cpp
58

if you create a local

auto pluginController = core()->pluginController();

you can use it here and below, somehwat reducing the line lengths

do it at your own will

79

it's empty, so why qDeleteAll? it won't have any effect

101

remove

102

instead for readbility wrap the code below into else here

This revision now requires changes to proceed.Feb 12 2017, 11:47 AM
antonanikin edited edge metadata.
  • Fix inline comments
antonanikin marked 2 inline comments as done.Feb 12 2017, 2:15 PM
antonanikin added inline comments.
debuggers/gdb/debuggerplugin.cpp
58

Good idea :)

antonanikin marked an inline comment as done.
antonanikin edited edge metadata.
  • Small codestyle fix
  • Fix inline comments
antonanikin marked 2 inline comments as done.Feb 13 2017, 1:51 AM
mwolff accepted this revision.Feb 15 2017, 10:26 PM
mwolff edited edge metadata.

lgtm, thank you

This revision is now accepted and ready to land.Feb 15 2017, 10:26 PM
This revision was automatically updated to reflect the committed changes.