145712 - nsMsgRDFDataSource::Cleanup should not call nsIObserverService::removeObserver

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[timeless]

Comment 1

[Alec Flett]

This is a question, not a bug. File a bug when you know an answer to the question.

Comment 2

[timeless]

i spoke with alecf. we do want to change this.

Comment 3

[Alec Flett]

ok, but this is not my bug. Timeless, please at least take the time to explain,
for the record, why this is important, what impact it has, and so forth.

Comment 4

[[e-mail] (gone - use [e-mail] instead)]

Valid, invalid?  I just want to get this into or out of the system, either way.

Timeless, can you elaborate?

Comment 5

[timeless]

 61 nsMsgRDFDataSource::~nsMsgRDFDataSource()
 62 {
 65     if (mInitialized) Cleanup();
 66 }

 72 nsMsgRDFDataSource::Init()
 73 {
 80     nsCOMPtr<nsIObserverService> obs =
do_GetService("@mozilla.org/observer-service;1",
 81                                                      &rv);
 83     rv = obs->AddObserver(NS_STATIC_CAST(nsIObserver*, this),
NS_XPCOM_SHUTDOWN_OBSERVER_ID, PR_TRUE);
 91 }

116 NS_INTERFACE_MAP_BEGIN(nsMsgRDFDataSource)
120     NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
122 NS_INTERFACE_MAP_END

 94 void nsMsgRDFDataSource::Cleanup()
 95 {
100     nsCOMPtr<nsIObserverService> obs =
do_GetService("@mozilla.org/observer-service;1",
101                                                      &rv);
103         rv = obs->RemoveObserver(NS_STATIC_CAST(nsIObserver*, this),
104                                  NS_XPCOM_SHUTDOWN_OBSERVER_ID);
111 }

ownsWeak  : If set to false, the nsIObserverService will hold a strong reference
to |anObserver|. If set to true and |anObserver| supports the nsIWeakReference
interface, a weak reference will be held. Otherwise an error will be returned.

Ok, so what happens?
1. We're a weak observer. This means that anyone who looks for us will be
willing to deal with the fact that we're dead. We're not supposed to waste
cycles telling them that we're dead.
2. If we weren't a weak observer then there'd be a strong reference to us which
would mean our destructor could never be called since that's the place where
we'd unregister ourselves (infinite unreachable loop)

Comment 6

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) <nsMsgRDFDataSource.cpp> [Checkin: Comment 8]

Comment 7

[[e-mail]]

Comment on attachment 331247 [details] [diff] [review]
(Av1) <nsMsgRDFDataSource.cpp>
[Checkin: Comment 8]

Well I'm tempted to say that the destructor shouldn't be trying to clean up but I guess this is the safer fix.

Comment 8

[Serge Gautherie (:sgautherie)]

Comment on attachment 331247 [details] [diff] [review]
(Av1) <nsMsgRDFDataSource.cpp>
[Checkin: Comment 8]


http://hg.mozilla.org/comm-central/rev/417083743e7e