558452 - crash [@ nsImapProtocol::SetupWithUrl ][@ nsImapProtocol::SetupWithUrl] with null m_runningUrl - aURL isn't nsIImapUrl

Status: NEW

(MailNews Core :: Networking: IMAP, defect)

Description

[Wayne Mery (:wsmwk)]

+++ This bug was initially created as a clone of Bug #507088 +++

bp-49c34c1a-a481-46e7-a00d-7362b2100105 (phorton)
0	thunderbird.exe	nsImapProtocol::SetupWithUrl	 mailnews/imap/src/nsImapProtocol.cpp:742
1	thunderbird.exe	nsImapProtocol::LoadImapUrl	mailnews/imap/src/nsImapProtocol.cpp:1979
2	thunderbird.exe	nsImapIncomingServer::LoadNextQueuedUrl	mailnews/imap/src/nsImapIncomingServer.cpp:537
3	xpcom_core.dll	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
4	xpcom_core.dll
5	xpcom_core.dll	nsProxyObjectCallInfo::Run	xpcom/proxy/src/nsProxyEvent.cpp:181 

no crashes found with comments

Comment 1

[timeless]

nsresult nsImapProtocol::SetupWithUrl(nsIURI * aURL, nsISupports* aConsumer)
742 m_runningUrl->GetMockChannel(getter_AddRefs(m_mockChannel)); 

m_runningUrl is null

Comment 2

[Magnus Melin [:mkmelin]]

Would appear that way, but how could it when we have this a few lines up

711 m_runningUrl = do_QueryInterface(aURL, &rv);
712 if (NS_FAILED(rv)) return rv;

Comment 3

[timeless]

well, either the code is threaded, or one of the intermediate lines calls something interesting or calls out to something which reenters something interestng.

Comment 4

[Ludovic Hirlimann [:Usul]]

So I got that one after a week-end where I had left the computer sleeping.

Comment 5

[Ludovic Hirlimann [:Usul]]

Again on waking the computer up !!

Comment 6

[David :Bienvenu]

Ludo, do you have a new crash-stat link?

Comment 7

[Ludovic Hirlimann [:Usul]]

(In reply to comment #6)
> Ludo, do you have a new crash-stat link?

Yes : http://crash-stats.mozilla.com/report/index/bp-c63079a8-0057-4592-8a72-a810c2100708

Comment 9

[Wayne Mery (:wsmwk)]

also @0x0 | nsImapProtocol::SetupWithUrl(nsIURI*, nsISupports*) on windows? bp-9cb5f8db-453d-4357-a677-501502100723

Comment 10

[Makoto Kato [:m_kato]]

Wayne, Bug 582165 is difference crash even if crash function is same.

Comment 11

[Wayne Mery (:wsmwk)]

I just crashed bp-ec6aa5ee-496c-444f-a6a4-d1ad72120308 but no protocol log :(

Comment 12

[Wayne Mery (:wsmwk)]

~top 140 crash.  I think a good bug to revisit after TB17 releases.


(In reply to Ludovic Hirlimann [:Usul] from comment #4)
> So I got that one after a week-end where I had left the computer sleeping.

bp-29a89b11-a814-4a5d-9468-d57ca2121031 says same thing - coming back from sleep. 
my comment 11 (20120302030054 13.0a1) probably also was after wake from sleep.
both point at  m_runningUrl->GetMockChannel  (ref: comment 1 and comment 2)


perhaps a different crash - bp-e759bab7-fc5f-4934-a293-734392121028  and bp-a9bf02a8-716d-4a8f-9ecc-bb6272121110 hit a different area of code -- http://hg.mozilla.org/releases/comm-release/annotate/ddf904aa1957/mailnews/imap/src/nsImapProtocol.cpp#l8085
  8085  return m_imapProtocolSink->GetUrlWindow(mailnewsUrl, aMsgWindow);
in nsImapProtocol::GetMsgWindow.  Which, coincidentally, has crashes with (rare) signature nsImapProtocol::GetMsgWindow(nsIMsgWindow**) as in bp-24d57332-6420-4727-b64b-146e32121110 and nsImapProtocol::GetMsgWindow as in bp-70a52ef7-0b50-4dc3-915c-3e2a22121106

Comment 13

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

There's already plenty of if (m_runningUrl) checks in the file, also later in the same method...

Comment 14

[:Irving Reid (No longer working on Firefox)]

Comment on attachment 680458 [details] [diff] [review]
proposed fix

Review of attachment 680458 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/imap/src/nsImapProtocol.cpp
@@ +673,5 @@
>      }
>      nsCOMPtr<nsIImapIncomingServer> imapServer = do_QueryInterface(server);
>      nsCOMPtr<nsIStreamListener> aRealStreamListener = do_QueryInterface(aConsumer);
> +    if (m_runningUrl)
> +      m_runningUrl->GetMockChannel(getter_AddRefs(m_mockChannel));

Would it make more sense to exit early (around line 644) if m_runningUrl is null? As far as I can tell, a null pointer here implies we're trying to run a non-IMAP URL, which doesn't make sense.

Comment 15

[Magnus Melin [:mkmelin]]

Comment on attachment 680458 [details] [diff] [review]
proposed fix

I think that's done already, no? (rv would be failed). Otherwise we'd at least crash with mailnewsUrl - http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapProtocol.cpp#644

So something is happening in between that nulls out m_runningURL - i don't know the larger picture, but i assume it's a threading issue.

Comment 16

[:Irving Reid (No longer working on Firefox)]

(In reply to Magnus Melin from comment #15)
> Comment on attachment 680458 [details] [diff] [review]
> proposed fix
> 
> I think that's done already, no? (rv would be failed). Otherwise we'd at
> least crash with mailnewsUrl -
> http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapProtocol.
> cpp#644
> 
> So something is happening in between that nulls out m_runningURL - i don't
> know the larger picture, but i assume it's a threading issue.

Can you look into where else in the code we mess with m_runningURL and why, to see if we can understand and/or prevent the race condition?

Comment 17

[Magnus Melin [:mkmelin]]

Well there is at least a nulling out in nsImapProtocol::ReleaseUrlState http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapProtocol.cpp#973

But other than that, i can't really tell.

Comment 18

[:Irving Reid (No longer working on Firefox)]

Comment on attachment 680458 [details] [diff] [review]
proposed fix

David, I can't make sense of the threading in this bug. Can you take a look at it?

Comment 19

[Merike Sell (:merike)]

First time ever I got it. But right before getting https://crash-stats.mozilla.com/report/index/7ce8f2d8-8930-43b6-90f7-034d92131202 this was happening:

2013-12-02 16:25:44.464719 UTC - -2019558592[7634da80]: ReadNextLine [stream=765b0220 nb=10 needmore=0]
2013-12-02 16:25:44.464740 UTC - -2019558592[7634da80]: 888d8800:imap.gmail.com:S-[Gmail]/Stellarium:CreateNewLineFromSocket: + idling
2013-12-02 16:35:17.204418 UTC - -1220475136[b7222240]: proposed url = INBOX folder for connection INBOX has To Wait = FALSE can run = TRUE
2013-12-02 16:35:17.204692 UTC - 1968171840[71372c00]: 713de000:imap.gmail.com:S-INBOX:SendData: DONE
2013-12-02 16:35:17.205045 UTC - -1220475136[b7222240]: proposed url = [Gmail]/Stellarium folder for connection INBOX has To Wait = FALSE can run = FALSE
2013-12-02 16:35:17.205103 UTC - -1220475136[b7222240]: proposed url = [Gmail]/Stellarium folder for connection [Gmail]/Stellarium has To Wait = FALSE can run = TRUE
2013-12-02 16:35:20.000033 UTC - 1968171840[71372c00]: ReadNextLine [stream=71370460 nb=34 needmore=0]
2013-12-02 16:35:20.000064 UTC - 1968171840[71372c00]: 713de000:imap.gmail.com:S-INBOX:CreateNewLineFromSocket: 252 OK IDLE terminated (Success)
2013-12-02 16:35:20.000145 UTC - 1968171840[71372c00]: 713de000:imap.gmail.com:S-INBOX:ProcessCurrentURL: entering
2013-12-02 16:35:20.000197 UTC - 1968171840[71372c00]: 713de000:imap.gmail.com:S-INBOX:ProcessCurrentURL:imap://[e-mail]:993/select%3E/INBOX:  = currentUrl

Also, right after crashing netstat had 5 established and 1 close_wait connection to la-in-f108.1e100.net:imaps and one imap2 established connection to a non-google host. And given the time it happened, I was accessing my google apps email account and marking email read/unread on my Android phone at the same time.

Comment 20

[Phil Pishioneri]

My crash of version 24.3.0: bp-fdd983bd-b3ce-4719-a617-77c122140220

Always-on desktop, no wake up from sleep involved. No protocol logs, and the event happened while I was away from screen, so no user-interaction. Thunderbird is polling IMAP and using IDLE.

Comment 21

[Phil Pishioneri]

Had another crash overnight: bp-ae8a8471-9bb2-48c8-bd0f-a6ccd2140221.

Having not had crashes of this signature before, I'll note that my Thunderbird configuration changed recently with the addition of another IMAP server.

Comment 22

[Phil Pishioneri]

I've had a few more crashes. For the most recent, bp-ae42adde-4537-49ee-b19e-dbf3e2140227, I also have a protocol log (IMAP:5,ImapAutoSync:5,timestamp) from start to crash of 187,260,369 bytes (22039034 gzipped), covering about 5 hours.

I'm reticent to attach the log to the bug (plus it may be too large), but can make it available to a developer.

Comment 23

[Ludovic Hirlimann [:Usul]]

you can probably send the log to magnus who's working on this if he needs it.

Comment 24

[Wayne Mery (:wsmwk)]

(In reply to :Irving Reid from comment #18)
> Comment on attachment 680458 [details] [diff] [review]
> proposed fix
> 
> David, I can't make sense of the threading in this bug. Can you take a look
> at it?

Irving, Bienvenu isn't going to touch this unless it's absolutely essential and we contact him outside of bugzilla.  Do you want to PM bienvenu, or do we have someone (rkent?) who might be able to sort it out?

Comment 25

[Wayne Mery (:wsmwk)]

magnus, did you get Phil's protocol log?

FWIW, Phil is still crashing recently ... bp-8ff9fbe2-0e67-4f06-aa43-07cd82140825 bp-1b36d76c-de93-45c7-bc42-62e5c2140828

Comment 26

[Magnus Melin [:mkmelin]]

Not that I recall, but I don't think it would help much.

Comment 27

[Phil Pishioneri]

I did not make a log available. I can, but the most recent one I have is from version 24.3.0; let me know if that's usable. I've enabled logging again to collect more recent data.  FYI, I've been logging with

NSPR_LOG_MODULES="IMAP:5,ImapAutoSync:5,timestamp"

if you need different settings, let me know. I'm also willing to run a debug build.

Comment 28

[:Irving Reid (No longer working on Firefox)]

Wayne, at this point I'll leave it to Magnus or whoever else has time to investigate.

Comment 29

[Wayne Mery (:wsmwk)]

FWIW, Phil's crashes account for 25% of crash reports with an email address. :)
wirelessgregg has another 25%.

Comment 30

[Wayne Mery (:wsmwk)]

Comment on attachment 680458 [details] [diff] [review]
proposed fix

This will need a different reviewer

Comment 31

[Phil Pishioneri]

I had two crashes this morning against version 31.1.2: crash bp-74f8232d-ed82-4d5a-91e9-ef5c12140929 and crash bp-0b0868e4-a013-4bcb-a115-77f682140929. Both have logs that I can make available (5MB, 1.5MB).

Comment 32

[Ludovic Hirlimann [:Usul]]

Removing myslef on all the bugs I'm cced on. Please NI me if you need something on MailNews Core bugs from me.

Comment 33

[Wayne Mery (:wsmwk)]

(In reply to Phil Pishioneri from comment #31)
> I had two crashes this morning against version 31.1.2: crash
> bp-74f8232d-ed82-4d5a-91e9-ef5c12140929 and crash
> bp-0b0868e4-a013-4bcb-a115-77f682140929. Both have logs that I can make
> available (5MB, 1.5MB).

PHil, are you still seeing crashes like two years ago?
From you in the last 2 months I see only 
bp-9290b418-1bd9-4346-aeb1-f345e2151201	2015-12-01 22:26:51 	PORT_ArenaAlloc_Util | nssTrust_GetCERTCertTrustForCert | stan_GetCERTCertificate   
bp-318c0a58-fe55-487a-859c-2003a2160128	2016-01-28 14:56:50 	shutdownhang | libsystem_kernel.dylib@0x16eb2

Comment 34

[Phil Pishioneri]

I have not seen the crash in quite a long time. That time may have been when I switched to a different IMAP service: but whether that's why or not, I don't get them.

Comment 35

[Wayne Mery (:wsmwk)]

afaict no change over last 6  months, and in any v60 version, despite patches in bug 825513 and Bug 1507718 - crash in nsImapProtocol::GetMessageSize via nsIMAPBodyShell::Generate. m_hostSessionList is stale

not much hope for a testcase.
sampling 10 crashes from 60.4.0 - two have McAfee installed, one bitdefender.  Of the remaining crashes 
* two haven't had othe rrecent crashes (previous crashes 7 weeks ago and one year)
* hard to say if there is a common theme in the stacks:

- three emanate from nsImapMailFolder::InitiateAutoSync bp-3a96030a-f232-4246-a080-062ce0190105
 0 	xul.dll	nsImapProtocol::SetupWithUrl(nsIURI*, nsISupports*)	comm/mailnews/imap/src/nsImapProtocol.cpp:723
1 	xul.dll	nsImapProtocol::LoadImapUrl(nsIURI*, nsISupports*)	comm/mailnews/imap/src/nsImapProtocol.cpp:2151
2 	xul.dll	nsImapIncomingServer::GetImapConnectionAndLoadUrl(nsIImapUrl*, nsISupports*)	comm/mailnews/imap/src/nsImapIncomingServer.cpp:455
3 	xul.dll	nsImapService::GetImapConnectionAndLoadUrl(nsIImapUrl*, nsISupports*, nsIURI**)	comm/mailnews/imap/src/nsImapService.cpp:2227
4 	xul.dll	nsImapService::FolderCommand(nsIMsgFolder*, nsIUrlListener*, char const*, int, nsIMsgWindow*, nsIURI**)	comm/mailnews/imap/src/nsImapService.cpp:1511
5 	xul.dll	nsImapService::UpdateFolderStatus(nsIMsgFolder*, nsIUrlListener*, nsIURI**)	comm/mailnews/imap/src/nsImapService.cpp:1563
6 	xul.dll	nsImapMailFolder::UpdateStatus(nsIUrlListener*, nsIMsgWindow*)	comm/mailnews/imap/src/nsImapMailFolder.cpp:1420
7 	xul.dll	nsImapMailFolder::InitiateAutoSync(nsIUrlListener*)	comm/mailnews/imap/src/nsImapMailFolder.cpp:9575
8 	xul.dll	nsAutoSyncManager::TimerCallback(nsITimer*, void*)	comm/mailnews/imap/src/nsAutoSyncManager.cpp:332 

- two are like bp-63496f75-75ce-46c1-acde-c0a8d0190104
 0 	xul.dll	nsImapProtocol::SetupWithUrl(nsIURI*, nsISupports*)	comm/mailnews/imap/src/nsImapProtocol.cpp:723
1 	xul.dll	nsImapProtocol::LoadImapUrl(nsIURI*, nsISupports*)	comm/mailnews/imap/src/nsImapProtocol.cpp:2151
2 	xul.dll	nsImapIncomingServer::GetImapConnectionAndLoadUrl(nsIImapUrl*, nsISupports*)	comm/mailnews/imap/src/nsImapIncomingServer.cpp:455
3 	xul.dll	nsImapService::GetImapConnectionAndLoadUrl(nsIImapUrl*, nsISupports*, nsIURI**)	comm/mailnews/imap/src/nsImapService.cpp:2227
4 	xul.dll	nsImapService::FolderCommand(nsIMsgFolder*, nsIUrlListener*, char const*, int, nsIMsgWindow*, nsIURI**)	comm/mailnews/imap/src/nsImapService.cpp:1511
5 	xul.dll	nsImapService::UpdateFolderStatus(nsIMsgFolder*, nsIUrlListener*, nsIURI**)	comm/mailnews/imap/src/nsImapService.cpp:1563
6 	xul.dll	nsImapMailFolder::UpdateStatus(nsIUrlListener*, nsIMsgWindow*)	comm/mailnews/imap/src/nsImapMailFolder.cpp:1420
7 	xul.dll	nsImapIncomingServer::OnStopRunningUrl(nsIURI*, nsresult)	comm/mailnews/imap/src/nsImapIncomingServer.cpp:2427
8 	xul.dll	nsMsgMailNewsUrl::SetUrlState(bool, nsresult)	comm/mailnews/base/util/nsMsgMailNewsUrl.cpp:164
9 	xul.dll	nsImapMailFolder::SetUrlState(nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, nsresult)	comm/mailnews/imap/src/nsImapMailFolder.cpp:6846
10 	xul.dll	`anonymous namespace'::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, enum nsresult>::Run	comm/mailnews/imap/src/nsSyncRunnableHelpers.cpp:251

Comment 36

[Wayne Mery (:wsmwk)]

Ben, Perhaps related to the rats nest you are investigating.

Comment 37

[Wayne Mery (:wsmwk)]

Some, but not all, are during autosync
bp-5b7f81a1-d2f2-43eb-8225-551230191117

0 xul.dll nsImapProtocol::SetupWithUrl(nsIURI*, nsISupports*) comm/mailnews/imap/src/nsImapProtocol.cpp:785
1 xul.dll nsImapProtocol::LoadImapUrl(nsIURI*, nsISupports*) comm/mailnews/imap/src/nsImapProtocol.cpp:2100
2 xul.dll nsImapIncomingServer::GetImapConnectionAndLoadUrl(nsIImapUrl*, nsISupports*) comm/mailnews/imap/src/nsImapIncomingServer.cpp:418
3 xul.dll nsImapService::GetImapConnectionAndLoadUrl(nsIImapUrl*, nsISupports*, nsIURI**) comm/mailnews/imap/src/nsImapService.cpp:2083
4 xul.dll nsImapService::FolderCommand(nsIMsgFolder*, nsIUrlListener*, char const*, int, nsIMsgWindow*, nsIURI**) comm/mailnews/imap/src/nsImapService.cpp:1428
5 xul.dll nsImapService::UpdateFolderStatus(nsIMsgFolder*, nsIUrlListener*, nsIURI**) comm/mailnews/imap/src/nsImapService.cpp:1474
6 xul.dll nsImapMailFolder::UpdateStatus(nsIUrlListener*, nsIMsgWindow*) comm/mailnews/imap/src/nsImapMailFolder.cpp:1301
7 xul.dll nsImapMailFolder::InitiateAutoSync(nsIUrlListener*) comm/mailnews/imap/src/nsImapMailFolder.cpp:8758

Comment 38

[Wayne Mery (:wsmwk)]

Phil, do you still crash ?

Comment 39

[Wayne Mery (:wsmwk)]

I have one user who reported crashing with Bullguard AV. I checked five recent crashes and none have Bullguard.

I checked about 8 crashes and actually most don't have AV so I don't think there is a strong AV correlation.
bp-85ecd0cd-ecfc-4caa-87e2-c04250210306 AVG
bp-852d2aa4-f01d-45bf-bdb3-20be40210305 Trend micro

Comment 40

[Wayne Mery (:wsmwk)]

Looks like version 91 crash rate was roughly the same as version 78 (in early November 2021 just before crash-stats stopped accepting TB crash reports)

Crash report: https://crash-stats.mozilla.org/report/index/561aa5bc-6376-48e7-a868-28c700211109 91.3.0

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0 xul.dll nsImapProtocol::SetupWithUrl comm/mailnews/imap/src/nsImapProtocol.cpp:793
1 xul.dll nsImapProtocol::LoadImapUrl comm/mailnews/imap/src/nsImapProtocol.cpp:2226
2 xul.dll nsImapIncomingServer::GetImapConnectionAndLoadUrl comm/mailnews/imap/src/nsImapIncomingServer.cpp:424
3 xul.dll nsImapService::GetImapConnectionAndLoadUrl comm/mailnews/imap/src/nsImapService.cpp:2087
4 xul.dll nsImapService::FolderCommand comm/mailnews/imap/src/nsImapService.cpp:1429
5 xul.dll nsImapService::UpdateFolderStatus comm/mailnews/imap/src/nsImapService.cpp:1475
6 xul.dll nsImapMailFolder::UpdateStatus comm/mailnews/imap/src/nsImapMailFolder.cpp:1311
7 xul.dll nsImapMailFolder::InitiateAutoSync comm/mailnews/imap/src/nsImapMailFolder.cpp:8697
8 xul.dll static nsAutoSyncManager::TimerCallback comm/mailnews/imap/src/nsAutoSyncManager.cpp:299
9 xul.dll nsTimerImpl::Fire xpcom/threads/nsTimerImpl.cpp:618

Comment 41

[Wayne Mery (:wsmwk)]

bp-780009f7-9923-481d-a3d4-a480b0230814 version 115

Comment 42

[Wayne Mery (:wsmwk)]

Looks like version 115 crash rate might end up a bit lower. But won't know until October after full updates are complete.
https://crash-stats.mozilla.org/signature/?signature=nsImapProtocol%3A%3ASetupWithUrl&date=%3E%3D2023-02-28T07%3A18%3A00.000Z&date=%3C2023-08-30T07%3A18%3A00.000Z#summary
bp-50d8fe57-0054-4adf-af60-fddf10230828. 117.0b1

Comment 43

[Wayne Mery (:wsmwk)]

There are a few v152 and 140esr crashes, so I assume the crashing will still show in 153.