[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Problem with appletRemoved signal
From: Alexis_Ménard <menard () kde ! org>
Date: 2009-01-26 14:21:58
Message-ID: 81941aea0901260621u3a105809g17d7e440aa3877a2 () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Hello folks,
There is a problem with appletRemoved public signal in containment. Here is
the signature :
/**
* This signal is emitted when an applet is destroyed
*/
void appletRemoved(Plasma::Applet *applet);
So a containment that is connected to this signal expect an Applet pointer.
But who emit this signal? Our beloved base class Containment which is
connected to the QObject destroyed signal of the applet.
Here is the slot :
void ContainmentPrivate::appletDestroyed(QObject *object);
When we arrive in the this slot applet destructor has been already called.
Since we do a static_cast just because we need the value of the pointer,
it's ok. BUT after we call :
emit q->appletRemoved(applet); with the pointer we get after the
static_cast.
Aie aie aie. Valgrind complain and we never know, people can use the pointer
to do some stuff when they get the appletRemoved signal, (we already do that
in our panel.cpp class). Luckily it doesn't crash but it can happen.
I have attach a patch that create a appletDestroyed signal that is emit just
after entering in Applet destructor. I wanted to use destroyed but we have
already a method called like this.
I guess if people agreed that i should backport it to 4.2 branch.
Is it OK?
PS : Sorry for don't using the review board but it doesn't work with git, i
don't have the svn revision.
[Attachment #5 (text/html)]
Hello folks,<br><br><br>There is a problem with appletRemoved public signal=
in containment. Here is the signature :<br><br>/**<br> * This signal =
is emitted when an applet is destroyed<br> */<br> void appletRemo=
ved(Plasma::Applet *applet);<br>
<br>So a containment that is connected to this signal expect an Applet poin=
ter. But who emit this signal? Our beloved base class Containment which is =
connected to the QObject destroyed signal of the applet.<br><br>Here is the=
slot :<br>
void ContainmentPrivate::appletDestroyed(QObject *object);<br><br>When we a=
rrive in the this slot applet destructor has been already called. Since we =
do a static_cast just because we need the value of the pointer, it's ok=
. BUT after we call :<br>
<br>emit q->appletRemoved(applet); with the pointer we get after the sta=
tic_cast. <br><br>Aie aie aie. Valgrind complain and we never know, people =
can use the pointer to do some stuff when they get the appletRemoved signal=
, (we already do that in our panel.cpp class). Luckily it doesn't crash=
but it can happen.<br>
<br>I have attach a patch that create a appletDestroyed signal that is emit=
just after entering in Applet destructor. I wanted to use destroyed but we=
have already a method called like this.<br><br>I guess if people agreed th=
at i should backport it to 4.2 branch.<br>
<br>Is it OK?<br><br>PS : Sorry for don't using the review board but it=
doesn't work with git, i don't have the svn revision.<br>
--00163646be022b2c9d0461637691--
["plasma.diff" (text/x-patch)]
diff --git a/plasma/applet.cpp b/plasma/applet.cpp
index 0956fdf..b4116af 100644
--- a/plasma/applet.cpp
+++ b/plasma/applet.cpp
@@ -135,6 +135,9 @@ Applet::Applet(QObject *parentObject, const QVariantList &args)
Applet::~Applet()
{
+ //let people know that i will die
+ emit appletDestroyed(this);
+
if (!d->transient && d->extender) {
//This would probably be nicer if it was located in extender. But in it's \
dtor, this won't
//work since when that get's called, the applet's config() isn't accessible \
anymore. (same
diff --git a/plasma/applet.h b/plasma/applet.h
index 5f4ad71..9d5324d 100644
--- a/plasma/applet.h
+++ b/plasma/applet.h
@@ -611,6 +611,11 @@ class PLASMA_EXPORT Applet : public QGraphicsWidget
*/
void activate();
+ /**
+ * Emitted when the applet is deleted
+ */
+ void appletDestroyed(Plasma::Applet *applet);
+
public Q_SLOTS:
/**
* Sets the immutability type for this applet (not immutable,
diff --git a/plasma/containment.cpp b/plasma/containment.cpp
index 9cc469b..73d5a7b 100644
--- a/plasma/containment.cpp
+++ b/plasma/containment.cpp
@@ -735,7 +735,7 @@ void Containment::addApplet(Applet *applet, const QPointF &pos, \
bool delayInit)
connect(applet, SIGNAL(configNeedsSaving()), this, SIGNAL(configNeedsSaving()));
connect(applet, SIGNAL(releaseVisualFocus()), this, \
SIGNAL(releaseVisualFocus()));
- connect(applet, SIGNAL(destroyed(QObject*)), this, \
SLOT(appletDestroyed(QObject*))); + connect(applet, \
SIGNAL(appletDestroyed(Plasma::Applet*)), this, \
SLOT(appletDestroyed(Plasma::Applet*))); connect(applet, SIGNAL(activate()), this, \
SIGNAL(activate()));
if (pos != QPointF(-1, -1)) {
@@ -1698,16 +1698,8 @@ bool ContainmentPrivate::regionIsEmpty(const QRectF ®ion, \
Applet *ignoredAppl return true;
}
-void ContainmentPrivate::appletDestroyed(QObject *object)
+void ContainmentPrivate::appletDestroyed(Plasma::Applet *applet)
{
- // we do a static_cast here since it really isn't an Applet by this
- // point anymore since we are in the qobject dtor. we don't actually
- // try and do anything with it, we just need the value of the pointer
- // so this unsafe looking code is actually just fine.
- //
- // NOTE: DO NOT USE THE applet VARIABLE FOR ANYTHING OTHER THAN COMPARING
- // THE ADDRESS! ACTUALLY USING THE OBJECT WILL RESULT IN A CRASH!!!
- Applet *applet = static_cast<Plasma::Applet*>(object);
applets.removeAll(applet);
if (focusedApplet == applet) {
focusedApplet = 0;
diff --git a/plasma/containment.h b/plasma/containment.h
index 354ccb7..e234341 100644
--- a/plasma/containment.h
+++ b/plasma/containment.h
@@ -507,7 +507,7 @@ class PLASMA_EXPORT Containment : public Applet
const QGraphicsItem *toolBoxItem() const;
private:
- Q_PRIVATE_SLOT(d, void appletDestroyed(QObject*))
+ Q_PRIVATE_SLOT(d, void appletDestroyed(Plasma::Applet*))
Q_PRIVATE_SLOT(d, void containmentAppletAnimationComplete(QGraphicsItem \
*item,
\
Plasma::Animator::Animation anim)) Q_PRIVATE_SLOT(d, void triggerShowAddWidgets())
diff --git a/plasma/private/containment_p.h b/plasma/private/containment_p.h
index 6fe21b7..0a91b98 100644
--- a/plasma/private/containment_p.h
+++ b/plasma/private/containment_p.h
@@ -75,7 +75,7 @@ public:
void positionContainments();
void setLockToolText();
void handleDisappeared(AppletHandle *handle);
- void appletDestroyed(QObject*);
+ void appletDestroyed(Plasma::Applet*);
void containmentAppletAnimationComplete(QGraphicsItem *item, \
Plasma::Animator::Animation anim); void zoomIn();
void zoomOut();
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic