Closed Bug 380167 Opened 17 years ago Closed 5 years ago

crash in [@ nsMsgDBFolder::NotifyItemRemoved] because mailnews doesn't use proper references deleting mail/messages

Categories

(MailNews Core :: Database, defect)

x86
All
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: arno, Unassigned)

References

Details

(Keywords: crash, Whiteboard: [rare][fixed by bug 409458])

Crash Data

in a thunderbird extension, I have the following code:

var crashFolderListener = {
    
    QueryInterface: function(iid) {
        dump("QueryInterface" + iid + "\n");
        if (iid.equals(Components.interfaces.nsIFolderListener) ||
            iid.equals(Components.interfaces.nsISupportsWeakReference) ||
            iid.equals(Components.interfaces.nsISupports))
            return this;
        throw Components.results.NS_NOINTERFACE;
    },
     
     OnItemAdded: function(parentItem, item) {
         dump("OnItemAdded\n");
         },
     OnItemBoolPropertyChanged: function(item, property, oldValue, newValue) {
         dump("OnItemBoolPropertyChanged\n");
         },
     OnItemEvent: function(item, event) {
         dump("OnItemEvent\n");
         },
     OnItemIntPropertyChanged: function(item, property, oldValue, newValue) {
         dump("OnItemIntPropertyChanged\n");
         },
     OnItemPropertyChanged: function(item, property, oldValue, newValue) {
         dump("OnItemPropertyChanged\n");
         },
     OnItemPropertyFlagChanged: function(item, property, oldFlag, newFlag) {
         dump("OnItemPropertyFlagChanged\n");
         },
     OnItemRemoved: function(parentItem, item) {
         dump("OnItemRemoved\n");
         },
     OnItemUnicharPropertyChanged: function(item, property, oldValue, newValue) {
         dump("OnItemUnicharPropertyChanged\n");
         }
}

function crashOnLoad() {
    var mboxUri = 'mailbox://xxxx@xxxx@xxxx/Inbox'; // my mailbox uri

    var rdf = Components.classes["@mozilla.org/rdf/rdf-service;1"].getService(Components.interfaces.nsIRDFService);
    var resource = RDF.GetResource(mboxUri);

    var folder = resource.QueryInterface(Components.interfaces.nsIMsgFolder);

    folder.AddFolderListener(crashFolderListener);

}

window.addEventListener("load", crashOnLoad, false);



As soon as I try to download messages, QueryInterface method crashFolderListener is called, and thunderbird crashes directly after. More precisely, it happens because NotifyItemRemoved is called.

I've set some printf in mailnews/base/util/nsMsgDBFolder.cpp, and it seems that the crash occurs on that very line:  listener->OnItemRemoved(this, item);

I also observed the same crash in OnItemIntPropertyChanged on the line  listener->OnItemIntPropertyChanged(this, property, oldValue, newValue); 
and I suspect the bug may occur on any method of the folderlistener

#0  0xb62bb79c in nsMsgDBFolder::NotifyItemRemoved () from /home/arno/thunderbird/dist/bin/components/libmail.so
#1  0xb62b4ef1 in nsMsgDBFolder::OnHdrAddedOrDeleted () from /home/arno/thunderbird/dist/bin/components/libmail.so
#2  0xb62b90d8 in nsMsgDBFolder::OnParentChanged () from /home/arno/thunderbird/dist/bin/components/libmail.so
#3  0xb63cf8ea in nsMsgDatabase::NotifyParentChangedAll () from /home/arno/thunderbird/dist/bin/components/libmail.so
#4  0xb63db67f in nsMsgThread::ReparentNonReferenceChildrenOf () from /home/arno/thunderbird/dist/bin/components/libmail.so
#5  0xb63db7d9 in nsMsgThread::RerootThread () from /home/arno/thunderbird/dist/bin/components/libmail.so
#6  0xb63dcafa in nsMsgThread::AddChild () from /home/arno/thunderbird/dist/bin/components/libmail.so
#7  0xb63d0e7d in nsMsgDatabase::AddToThread () from /home/arno/thunderbird/dist/bin/components/libmail.so
#8  0xb63d19fd in nsMsgDatabase::ThreadNewHdr () from /home/arno/thunderbird/dist/bin/components/libmail.so
#9  0xb63ce00a in nsMsgDatabase::AddNewHdrToDB () from /home/arno/thunderbird/dist/bin/components/libmail.so
#10 0xb63622b6 in nsMsgMailboxParser::PublishMsgHeader () from /home/arno/thunderbird/dist/bin/components/libmail.so
#11 0xb63604af in nsMsgMailboxParser::HandleLine () from /home/arno/thunderbird/dist/bin/components/libmail.so
#12 0xb62b41dd in nsMsgLineBuffer::ConvertAndSendBuffer () from /home/arno/thunderbird/dist/bin/components/libmail.so
#13 0xb62b4a42 in nsMsgLineBuffer::BufferInput () from /home/arno/thunderbird/dist/bin/components/libmail.so
#14 0xb6361ddb in nsMsgMailboxParser::ProcessMailboxInputStream () from /home/arno/thunderbird/dist/bin/components/libmail.so
#15 0xb63601e4 in nsMsgMailboxParser::OnDataAvailable () from /home/arno/thunderbird/dist/bin/components/libmail.so
#16 0xb6366430 in nsMailboxProtocol::ReadFolderResponse () from /home/arno/thunderbird/dist/bin/components/libmail.so
#17 0xb636717d in nsMailboxProtocol::ProcessProtocolState () from /home/arno/thunderbird/dist/bin/components/libmail.so
#18 0xb62d551a in nsMsgProtocol::OnDataAvailable () from /home/arno/thunderbird/dist/bin/components/libmail.so
#19 0xb7162a55 in nsInputStreamPump::OnStateTransfer () from /home/arno/thunderbird/dist/bin/components/libnecko.so
#20 0xb7162b65 in nsInputStreamPump::OnInputStreamReady () from /home/arno/thunderbird/dist/bin/components/libnecko.so
#21 0xb7e34c25 in nsInputStreamReadyEvent::EventHandler () from /home/arno/thunderbird/dist/bin/libxpcom_core.so
#22 0xb7e47f57 in PL_HandleEvent () from /home/arno/thunderbird/dist/bin/libxpcom_core.so
#23 0xb7e48237 in PL_ProcessPendingEvents () from /home/arno/thunderbird/dist/bin/libxpcom_core.so
#24 0xb7e49dfd in nsEventQueueImpl::ProcessPendingEvents () from /home/arno/thunderbird/dist/bin/libxpcom_core.so
#25 0xb67bff45 in event_processor_callback () from /home/arno/thunderbird/dist/bin/components/libwidget_gtk2.so
#26 0xb777bc7f in g_io_channel_unix_get_fd () from /usr/lib/libglib-2.0.so.0
#27 0xb7752731 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#28 0xb77557a6 in g_main_context_check () from /usr/lib/libglib-2.0.so.0
#29 0xb7755b67 in g_main_loop_run () from /usr/lib/libglib-2.0.so.0
#30 0xb7bc0281 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
#31 0xb67c0345 in nsAppShell::Run () from /home/arno/thunderbird/dist/bin/components/libwidget_gtk2.so
#32 0xb6782ad7 in nsAppStartup::Run () from /home/arno/thunderbird/dist/bin/components/libtoolkitcomps.so
#33 0x0804f651 in XRE_main ()
#34 0x0804ae23 in main ()
This feels like a duplicate.
http://mxr.mozilla.org/seamonkey/source/mailnews/base/util/nsMsgDBFolder.h
 201   nsVoidArray mListeners; //This can't be an nsISupportsArray because due to
 202                           //ownership issues, listeners can't be AddRef'd

For now, you're going to need to change your code such that you have /something/ holding a strong reference to your object. You could register with the observerservice and ask to listen for xpcom-shutdown.

Somewhere, I have a patch that enables code to recognize javascript objects and complain (It'd protect you against the crash...).
Assignee: mscott → bienvenu
Component: General → MailNews: Database
Product: Thunderbird → Core
QA Contact: general → database
Summary: crash in nsMsgDBFolder::NotifyItemRemoved → crash in [@ nsMsgDBFolder::NotifyItemRemoved] because mailnews doesn't use proper references
Product: Core → MailNews Core
(In reply to comment #1)
> This feels like a duplicate.
> http://mxr.mozilla.org/seamonkey/source/mailnews/base/util/nsMsgDBFolder.h
>  201   nsVoidArray mListeners; //This can't be an nsISupportsArray because due
> to
>  202                           //ownership issues, listeners can't be AddRef'd
> 
> For now, you're going to need to change your code such that you have
> /something/ holding a strong reference to your object. You could register with
> the observerservice and ask to listen for xpcom-shutdown.
> 
> Somewhere, I have a patch that enables code to recognize javascript objects and
> complain (It'd protect you against the crash...).

timeless, that would help extensions in general, right?  Since like it would be nice to have.
yes....
(In reply to comment #2)
> 
> timeless, that would help extensions in general, right?  Since[SEEMS] like it would be
> nice to have.
(In reply to comment #3)
> yes....

cool. will you file the bug if it isn't already? Or let me know if you want me to do somethin.
bienvenu, ref: comment 1 - do you think this is a duplicate?

examining ~10 crashes, I don't see any common add-ons in use. comments all mention deleting messages. #193 crash for v3.1.2

example crashes...

bp-35619614-723f-4e07-888b-bdd482100817 (u.b.muc)  v3.1.2
0	thunderbird.exe	nsMsgDBFolder::NotifyItemRemoved	 mailnews/base/util/nsMsgDBFolder.cpp:4877
1	thunderbird.exe	nsMsgDBFolder::OnHdrAddedOrDeleted	mailnews/base/util/nsMsgDBFolder.cpp:1080
2	thunderbird.exe	nsMsgDBFolder::OnHdrDeleted	mailnews/base/util/nsMsgDBFolder.cpp:1063
3	thunderbird.exe	nsMsgDatabase::NotifyHdrDeletedAll	mailnews/db/msgdb/src/nsMsgDatabase.cpp:724
4	thunderbird.exe	nsMsgDatabase::DeleteHeader	mailnews/db/msgdb/src/nsMsgDatabase.cpp:1810
5	thunderbird.exe	nsMsgDatabase::DeleteMessages	mailnews/db/msgdb/src/nsMsgDatabase.cpp:1753
6	thunderbird.exe	nsImapMailFolder::CopyMessagesOffline	mailnews/imap/src/nsImapMailFolder.cpp:7118
7	thunderbird.exe	nsImapMailFolder::CopyMessages	mailnews/imap/src/nsImapMailFolder.cpp:7276
8	thunderbird.exe	nsMsgCopyService::DoNextCopy	mailnews/base/src/nsMsgCopyService.cpp:321
9	thunderbird.exe	nsMsgCopyService::DoCopy	mailnews/base/src/nsMsgCopyService.cpp:263
10	thunderbird.exe	nsMsgCopyService::CopyMessages	mailnews/base/src/nsMsgCopyService.cpp:531 

bp-fb789f81-b3d3-42b1-9818-437c72100818 (arkon) v3.1.1
bp-9f6d7d17-39ce-45a9-ac8c-97c5f2100817 (harbant) v3.1
Status: NEW → UNCONFIRMED
Ever confirmed: false
OS: Linux → All
Summary: crash in [@ nsMsgDBFolder::NotifyItemRemoved] because mailnews doesn't use proper references → crash in [@ nsMsgDBFolder::NotifyItemRemoved(nsISupports*)] because mailnews doesn't use proper references deleting mail/messages
Status: UNCONFIRMED → NEW
Ever confirmed: true
yes, this is a duplicate, and timeless's analysis is correct.
(In reply to comment #6)
> yes, this is a duplicate, and timeless's analysis is correct.

I may have had the dup in my grip, but I've lost it. Perhaps someone else can find it
Whiteboard: dupme
Crash Signature: [@ nsMsgDBFolder::NotifyItemRemoved(nsISupports*)]
This appears to be EXTREMELY rare starting with version 5, i.e. pretty much gone after v3.1. So something got fixed

I find only two examples from the past ~10 
bp-fdb74b4e-0306-41f1-b308-5164b2120324 v12
bp-3fd4542d-133a-4e3f-bcb4-687c22111220 v8
Crash Signature: [@ nsMsgDBFolder::NotifyItemRemoved(nsISupports*)] → [@ nsMsgDBFolder::NotifyItemRemoved(nsISupports*)] [@ nsCOMPtr_base::assign_with_AddRef(nsISupports*) | nsMsgDBFolder::NotifyItemRemoved(nsISupports*) ] [@ nsCOMPtr_base::assign_with_AddRef | nsMsgDBFolder::NotifyItemRemoved ]Th
Whiteboard: dupme → [rare][dupeme?][fixed?]
Assignee: mozilla → nobody
many comments mention deleting messages.
some mention compact.
not as rare any more

bp-f463a95e-bc42-4275-aff4-5f71f2121024
bp-eebe71a8-3ef5-4533-80b0-d8af32120920
Crash Signature: [@ nsMsgDBFolder::NotifyItemRemoved(nsISupports*)] [@ nsCOMPtr_base::assign_with_AddRef(nsISupports*) | nsMsgDBFolder::NotifyItemRemoved(nsISupports*) ] [@ nsCOMPtr_base::assign_with_AddRef | nsMsgDBFolder::NotifyItemRemoved ]Th → [@ nsMsgDBFolder::NotifyItemRemoved(nsISupports*)] [@ nsCOMPtr_base::assign_with_AddRef(nsISupports*) | nsMsgDBFolder::NotifyItemRemoved(nsISupports*) ] [@ nsCOMPtr_base::assign_with_AddRef | nsMsgDBFolder::NotifyItemRemoved ] [@ nsMsgDBFolder::NotifyI…
Keywords: qawanted
Whiteboard: [rare][dupeme?][fixed?] → [rare]
I just noticed comment 1.  xref Bug 409458 - Crash when adding listener to local folder [@ nsMsgDBFolder::NotifyItemAdded(nsISupports*)

xref bug 458977
See Also: → 458977
Depends on: 409458
Does bug 409458 help us at all here?
Crash Signature: [@ nsMsgDBFolder::NotifyItemRemoved(nsISupports*)] [@ nsCOMPtr_base::assign_with_AddRef(nsISupports*) | nsMsgDBFolder::NotifyItemRemoved(nsISupports*) ] [@ nsCOMPtr_base::assign_with_AddRef | nsMsgDBFolder::NotifyItemRemoved ] [@ nsMsgDBFolder::NotifyI… → [@ nsCOMPtr_base::assign_with_AddRef | nsMsgDBFolder::NotifyItemRemoved ] [@ nsMsgDBFolder::NotifyItemRemoved ]
Flags: needinfo?(jorgk)
Summary: crash in [@ nsMsgDBFolder::NotifyItemRemoved(nsISupports*)] because mailnews doesn't use proper references deleting mail/messages → crash in [@ nsMsgDBFolder::NotifyItemRemoved] because mailnews doesn't use proper references deleting mail/messages
Good question. I can't answer it.
Flags: needinfo?(jorgk)

(In reply to Wayne Mery (:wsmwk) from comment #11)

Does bug 409458 help us at all here?

That was fixed in 60.3.2, and we have no crashes so far newer than 60.3.0. So I'm going to say this is fixed.

Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: qawanted
Resolution: --- → FIXED
Whiteboard: [rare] → [rare][fixed by bug 409458]
You need to log in before you can comment on or make changes to this bug.