Review Request 120152: Check for null session before using it (see description)

Olivier Jean de Gaalon olivier.jg at gmail.com
Fri Sep 12 05:19:39 UTC 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120152/
-----------------------------------------------------------

Review request for KDevelop, Aleix Pol Gonzalez, Milian Wolff, and Sergey Kalinichev.


Repository: kdevplatform


Description
-------

Currently TestItemRepositoryRegistryDeferredDeletion is crashing. This is because it creates and deletes the session, then in TestCore::shutdown an event loop is kicked off (just in case!) which loads some plugins (on shutdown!), and checks the active session to see if they are enabled.

Question #1: This is one fix, but it's not clear that this is even a valid situation (loading plugins after the session has died). Possibly the test should be fixed instead.

With this fix, the crash is deferred to KDevelop, where IDefinesAndIncludesManager::manager() Q_ASSERTS that it got a plugin, and then everywhere IDefinesAndIncludesManager::manager() is used, it's used without checking the pointer.

Question #2: Is it valid for IDefinesAndIncludesManager::manager() to Q_ASSERT that it got a plugin? There are a couple ways the plugin could be disabled, which would trigger this assert or just crash later in release mode.

After ensuring that the IDefinesAndIncludesManager::manager() pointer is checked, the test completes successfully.


Diffs
-----

  shell/plugincontroller.cpp 1acb54b 

Diff: https://git.reviewboard.kde.org/r/120152/diff/


Testing
-------

TestItemRepositoryRegistryDeferredDeletion


Thanks,

Olivier Jean de Gaalon

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140912/991dd9c4/attachment.html>


More information about the KDevelop-devel mailing list