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

[Olivier Jean de Gaalon]

> On Sept. 12, 2014, 1:55 p.m., Sergey Kalinichev wrote:
> > >Is it valid for IDefinesAndIncludesManager::manager() to Q_ASSERT that it got a plugin? 
> > 
> > I think yes, it's pretty much valid, as it uses X-KDevelop-LoadMode=AlwaysOn

Good point, the session doesn't even need to be involved to check. Fixed in 860d6d8e3768e663ddba5a60b5abf0552171ad5c


- Olivier Jean de


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


On Sept. 13, 2014, 1:47 p.m., Olivier Jean de Gaalon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120152/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2014, 1:47 p.m.)
> 
> 
> 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/20140913/6675e521/attachment.html>