> On June 6, 2011, 8:38 p.m., Aaron J. Seigo wrote:
> > i doubt this fix is correct. what looks more like is 
> > KStatusNotifierItemDBus::~KStatusNotifierItemDBus(). right now this is the 
> > implementation:
> > 
> > m_dbus.unregisterService(m_service);
> > 
> > and perhaps it should include:
> > 
> > m_dbus.unregisterObject("/StatusNotifierItem", this);
> > 
> > and perhaps even:
> > 
> > m_dbus.disconnectFromBus(m_service);
> > 
> > (though i expect the latter is done automatically when QDBusConnection is 
> > deleted? maybe not though...)
> 
> Shaun Reich wrote:
>     actually, looks like ~QDBusConnection() won't close the connection. 
> apparently disconnectFromBus does indeed need to be called explicitly. /me 
> suddenly wonders how much code doesn't do this, if need be.

Dunno if this class is using QDBusConnection::sessionBus(), but if it is then 
the disconnect is supposed to be automatic according to the sessionBus() API 
documentation. If indeed the disconnection is not occurring, then that might 
possibly explain the weird dbus related crashes reported in bug# 234484 as 
well. I hope that is not the case because I do not remember seeing anywhere 
where disconnectFromBus is called explicitly.


- Dawit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101527/#review3726
-----------------------------------------------------------


On June 6, 2011, 3:31 p.m., Christoph Feck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101527/
> -----------------------------------------------------------
> 
> (Updated June 6, 2011, 3:31 p.m.)
> 
> 
> Review request for kdelibs, Plasma and Marco Martin.
> 
> 
> Summary
> -------
> 
> According to bug 261180 there is a D-Bus connection leak in 
> KStatusNotifierItem. This patch potentially fixes it, but I do not know how 
> to test it.
> 
> 
> This addresses bug 261180.
>     http://bugs.kde.org/show_bug.cgi?id=261180
> 
> 
> Diffs
> -----
> 
>   kdeui/notifications/kstatusnotifieritem.cpp c48ed50 
> 
> Diff: http://git.reviewboard.kde.org/r/101527/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Christoph
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to