1768344 - Removing all saved passwords & attempting to shut down causes a hang and crash [@ shutdownhang | kernelbase.dll | mozilla::TaskControlleMr::GetRunnableForMTTask | nsThread::Shutdown | nsThreadManager::ShutdownNonMainThreads ] mozilla::SpinEventLoopUntil

Status: VERIFIED FIXED

(Thunderbird :: General, defect)

Description

[Arthur K. (he/him)]

This happened to me on my work PC the other day and repro'ed on my home PC as well.

STR with 101.0b1 (and now updated for 104.0b2):

1. Go to Tools > Settings > Privacy & Security > Passwords > Saved Passwords
2. Highlight all of your saved passwords and click Remove All (assuming here that you know your password(s) and can re-enter them again later)
3. Close the Saved Logins windows. Close the Settings tab
4. File > Exit to shut down Thunderbird
5. Open Task Manager to verify that one instance of TB is still not yet shut down

Expected Result:

1. No hang that leads to an eventual crash

Actual Result:

1. Hang that leads to an eventual crash

Crash: https://crash-stats.mozilla.org/report/index/b73ae5d3-b38d-48fd-8286-9b14d0220507

Comment 1

[Wayne Mery (:wsmwk)]

Antony, does this crash for you on Mac?

Comment 2

[Wayne Mery (:wsmwk)]

TCW, how reproducible is this for you?

I was not able to reproduce on Mac nor Windows with 101.0b2 (but I would very much love to)

Comment 3

[Arthur K. (he/him)]

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

TCW, how reproducible is this for you?

I was not able to reproduce on Mac nor Windows with 101.0b2 (but I would very much love to)

Apparently not reproducible on 101.b2. I just tried and TB just repeatedly bombarded me with login window prompts a second after clearing out passwords. I didn't see that behavior in 101.0b1. So for now and as of 101.b2, seems it's WFM.

Odd though that that same thing I did on my office PC and home PC caused the shutdown crash. Are you able to at least verify that there are two crashes on the same day? Those would have been mine.

Comment 4

[Arthur K. (he/him)]

This was the crash that happened on my work PC: https://crash-stats.mozilla.org/report/index/b73ae5d3-b38d-48fd-8286-9b14d0220507 same as my home PC.

Comment 5

[Antony]

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

Antony, does this crash for you on Mac?

Not seen any evidence and I do edit and change PWs a few times...

Comment 6

[Wayne Mery (:wsmwk)]

Robert can you reproduce using the steps in comment 0 on Windows ?

Comment 7

[Wayne Mery (:wsmwk)]

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

Robert can you reproduce using the steps in comment 0 on Windows ?

Walt, can you reproduce a crash with steps in comment 0?

Comment 8

[Robert Hartmann]

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

Robert can you reproduce using the steps in comment 0 on Windows ?

Hi, I cannot reproduce the crash on Windows 10 (64bit) using TB 91.11.0 (64-Bit)
and I cannot reproduce the crash using TB 102.0 (64-Bit) on Windows 10 (64bit)

Comment 9

[WaltS48 [:walts48]]

No, I cannot reproduce using 91.11.0 on Fedora Linux.

Comment 10

[Robert Hartmann]

I cannot reproduce the crash on Windows 10 (64 bit) using

• TB Daily 104.0a1 (2022-07-02) (64-bit)
• TB beta 103.0b2 (64-Bit)
  Best regards

Comment 11

[Wayne Mery (:wsmwk)]

Maybe bug 1629669 will help

Comment 12

[Arthur K. (he/him)]

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

Maybe bug 1629669 will help

Some similarities but not 100% 1:1. Could be caused by different things.

Comment 13

[Arthur K. (he/him)]

So I just tried to repro using 104.0b2 and my STR. I got this crash: https://crash-stats.mozilla.org/report/index/e0dca0f9-14f7-492e-b507-4e4e40220802

I am going to modify my STR a tad to reflect what I just did to repro.

Comment 14

[Arthur K. (he/him)]

I'm sure it probably won't make much difference but when I repro'ed the issue this time, it took TB a full 2m 45s from the time I did File > Exit until it disappeared from the screen. As before, one TB process was still running.

If I need to repro for you guys again, will I likely have any luck doing a perf profile? I'm guessing I will crash out before I can capture anything useful. But I can try if you want.

Comment 15

[Magnus Melin [:mkmelin]]

The STR didn't cause a crash for me on linux.

Comment 16

[Arthur K. (he/him)]

(In reply to Magnus Melin [:mkmelin] from comment #15)

The STR didn't cause a crash for me on linux.

Can't be my profile since I started from complete scratch early in Feb this year. I just tried to repro and TB is sitting in a state waiting to crash. While it is doing so, I tried to grab a perf profile for 5 seconds. This is it: https://share.firefox.dev/3Shqwo1

And when I blew away all the passwords to repro, I had Error Console open but nothing showed up.

Comment 17

[Arthur K. (he/him)]

Once again I was able to easily repro. Here's the crash: https://crash-stats.mozilla.org/report/index/be460cf3-c7d2-450f-87b3-9a9530220803

Comment 18

[Wayne Mery (:wsmwk)]

Walt, can you reproduce this on Windows? And Linux?

Comment 19

[WaltS48 [:walts48]]

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

Walt, can you reproduce this on Windows? And Linux?

I can not reproduce using Thunderbird 102.1.1 on Windows 10, or Fedora 35 Linux.

On Windows, I only had passwords for my POP3 account, and it's SMTP server.
Linux had those and passwords for my 4 IMAP accounts and my AIOE news account.

Comment 20

[Arthur K. (he/him)]

(In reply to WaltS48 [:walts48] from comment #19)

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

Walt, can you reproduce this on Windows? And Linux?

I can not reproduce using Thunderbird 102.1.1 on Windows 10, or Fedora 35 Linux.

On Windows, I only had passwords for my POP3 account, and it's SMTP server.
Linux had those and passwords for my 4 IMAP accounts and my AIOE news account.

So I am wondering how the heck I can repro with 100% ease and everyone else cannot. Could it be a setting I have turned on/off from the Settings menu? I'm curious if the fix in bug 1629669 will make it into 104.0b3 or b4? I can test again if it does.

Comment 21

[Arthur K. (he/him)]

This is how we look for 104.0b3: https://crash-stats.mozilla.org/report/index/b8f82bcb-876d-4398-9b27-05d510220810

Shows a slight signature change for 104.0b3. Only thing that changed between then and now is I am running 19045.1889 from yesterday's Patch Tuesday.

Comment 22

[Arthur K. (he/him)]

Well, I had high hopes for 104.0b4 but here is another crash I just repro'ed: https://crash-stats.mozilla.org/report/index/576cb0b4-b0da-4008-86e2-f2ac30220816

Comment 23

[Arthur K. (he/him)]

This is how we're looking for 105.0b1: https://crash-stats.mozilla.org/report/index/7ddc0d68-1b08-4731-82c0-57af60220825

Slight change between:
OLD: toolkit/xre/nsAppRunner.cpp:2060 > NEW: toolkit/xre/nsAppRunner.cpp:2069
OLD: mfbt/UniquePtr.h:275 > NEW: mfbt/UniquePtr.h:271
OLD: toolkit/xre/nsAppRunner.cpp:5914 > NEW: toolkit/xre/nsAppRunner.cpp:5940
OLD: toolkit/xre/nsAppRunner.cpp:5949 > NEW: toolkit/xre/nsAppRunner.cpp:5975
OLD: mail/app/nsMailApp.cpp:368 > NEW: mail/app/nsMailApp.cpp:389

Comment 24

[Arthur K. (he/him)]

So I am starting to suspect fully the password manager has some kind of bug because today something new happened. I woke my home PC from sleep and TB presented me with the username/password window as if it had forgotten them. Instead of entering the username and password and then proceeding to the "Allow TB to manage..." button, I simply closed the window using the [X] close button. I did a File > Exit action and TB went into a state of Not Responding to input. Task Manager showed it was still running but thinking about something. Eventually, it crashed: https://crash-stats.mozilla.org/report/index/9b3f9cac-4ab5-44c3-9060-9c9950220901

Signature is a bit different [@ shutdownhang | ZwWaitForAlertByThreadId | mozilla::TaskController::GetRunnableForMTTask | nsThread::Shutdown | nsThreadManager::ShutdownNonMainThreads ]

Comment 25

[Wayne Mery (:wsmwk)]

I am not able to reproduce this on Windows 10 using daily. But I am hopeful others can.

Can anyone else reproduce?

Comment 26

[Arthur K. (he/him)]

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

I am not able to reproduce this on Windows 10 using daily. But I am hopeful others can.

Can anyone else reproduce?

There's got to be something I have set on my instances of TB that allows me to repro with such ease.

This is how we're looking for 106.0b1: https://crash-stats.mozilla.org/report/index/41ba1bb4-6f87-4a48-adf4-21fd80220920

Signature has changed it seems: [@ shutdownhang | mozilla::OffTheBooksCondVar::Wait ]

Sadly I cannot add it to the bug title as it's now too long =\ Shall I open a new bug?

Comment 27

[Arthur K. (he/him)]

Bug 1790983 references this signature.

Comment 28

[Arthur K. (he/him)]

And this is with 106.0b5: https://crash-stats.mozilla.org/report/index/c0f48712-ffb1-4abe-987c-bf3990221011

Same @ shutdownhang | mozilla::OffTheBooksCondVar::Wait signature

Comment 29

[Gabriele Svelto [:gsvelto]]

This also looks like a duplicate of bug 1505660. The signature changes I'm doing in bug 1794587 will make the signatures in these bugs all the same.

Comment 30

[Arthur K. (he/him)]

The hang time, at least for me using my STR, has gotten progressively longer since 106. And with 107 it took around 4-5 minutes after a File > Exit action before TB finally crashed.

This is how we're looking for 107.0b1 build2: https://crash-stats.mozilla.org/report/index/09215680-e37b-454f-ba7d-165170221018
Still the same sig as in comment 26.

Comment 31

[Arthur K. (he/him)]

The only difference I see now is:

106.0b5
12 xul.dll XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:5934 cfi
13 xul.dll XRE_main(int, char**, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:5969 cfi

107.0b1
12 xul.dll XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:5937 cfi
13 xul.dll XRE_main(int, char**, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:5972 cfi

Comment 32

[Arthur K. (he/him)]

I got excited for a moment that this may have gotten fixed in 107.0b4 as I had removed two saved passwords (of four) from PW manager but it was a red herring. Took 4-5 minutes to eventually shut down and finally crash: https://crash-stats.mozilla.org/report/index/5184cdcd-5c31-4da9-800b-dee040221110

Signature seems a bit different though.

Comment 33

[Wayne Mery (:wsmwk)]

Yes, like bug 1790983, as of October 25 the signature will have changed. Thanks for checking it.

Comment 34

[Arthur K. (he/him)]

Just tested with the 108.0b1 build. Took around 4 minutes to crash but eventually did: https://crash-stats.mozilla.org/report/index/7af9cf9f-db02-4c4b-a145-6857f0221115

Comment 35

[ljcool2006]

For me, I don't remember removing any saved passwords when my crash happened.

Comment 36

[Arthur K. (he/him)]

Just tested with the 109.0b1 build. Took around 4 minutes to crash but eventually did: https://crash-stats.mozilla.org/report/index/948e4027-ab0b-4c83-93a4-0ef770221214

Comment 37

[Anje]

Could you provide some info on the following queries:

If you restart Windows OS in 'Safe mode with Networking' and start Thunderbird as normal, then do usual remove password and exit test, do you stll get same problem hang/crash?

In the 'profile name' folder, how many 'prefs-n.js' files do you see ? (Where 'n' is a number)

If you put Thunderbird into 'Offline' mode first, then remove passwords and exit TB, do you get same result ?

Comment 38

[Arthur K. (he/him)]

(In reply to Anje from comment #37)

Could you provide some info on the following queries:

If you restart Windows OS in 'Safe mode with Networking' and start Thunderbird as normal, then do usual remove password and exit test, do you stll get same problem hang/crash?

Yes: https://crash-stats.mozilla.org/report/index/7a7bb16f-fb35-48f9-992c-28a780221215

In the 'profile name' folder, how many 'prefs-n.js' files do you see ? (Where 'n' is a number)

There's only one prefs.js file.

If you put Thunderbird into 'Offline' mode first, then remove passwords and exit TB, do you get same result ?

Nope. TB shuts down instantly. So what does that tell us?

Comment 39

[Arthur K. (he/him)]

This is how we look with 110.0b1: https://crash-stats.mozilla.org/report/index/7e8e87eb-ef29-4770-9160-3ad4b0230118

Comment 40

[Arthur K. (he/him)]

And though I doubt it would have any clues in it, I had error console running while in a state of TB being stuck shutting down and this is what it captured:

10:36:24.370 mailnews.smtp:
error { target: TCPSocket, isTrusted: true, name: "NetworkTimeoutError", message: "Network", errorCode: 2152398862, srcElement: TCPSocket, currentTarget: TCPSocket, eventPhase: 2, bubbles: false, cancelable: false, … }
SmtpClient.jsm:438:17
_onError resource:///modules/SmtpClient.jsm:438
goQuitApplication chrome://global/content/globalOverlay.js:96
oncommand chrome://messenger/content/messenger.xhtml:1

10:36:43.910
<Provider> does not support changing store on the fly. It is most likely that you see this error because you updated to Redux 2.x and React Redux 2.x which no longer hot reload reducers automatically. See https://github.com/reactjs/react-redux/releases/tag/v2.0.0 for the migration instructions. react-redux.js:881:13
Redux 3
React 13
goQuitApplication chrome://global/content/globalOverlay.js:96
oncommand chrome://messenger/content/messenger.xhtml:1

Comment 41

[Wayne Mery (:wsmwk)]

Worth noting that blocking Bug 1505660 is a topcrash for Thunderbird 102.6.1, ranking #2, and ranks #1 for 109.0b4.

shutdownhang | nsThread::Shutdown | nsThreadManager::ShutdownNonMainThreads
application

Comment 42

[Arthur K. (he/him)]

So here's another possible clue it might be password manager or OAuth2 related. Today was my last day of work and promptly at 5PM they killed my email account access. Upon arriving home and starting TB, it of course complained that it couldn't log in. The incessant password/OAuth2 prompt kept coming up until I was able to go to Account > Server Settings > and uncheck all options there as well as disabling my calendar.

Until I did so, I kept getting barraged with the incessant password/OAuth2 prompt that wasn't able to be dismissed. As such I crashed twice: https://crash-stats.mozilla.org/report/index/b75cb7cf-7338-4f26-83cb-f68bd0230208
https://crash-stats.mozilla.org/report/index/6fafbe50-b243-49ef-93a8-b66a10230208

Comment 43

[Wayne Mery (:wsmwk)]

(In reply to Arthur K. (he/him) from comment #42)

So here's another possible clue it might be password manager or OAuth2 related.

Very possible.

I just crashed nightly bp-900ad218-a365-4f1b-b64a-2724e0230707 during a short startup that was needed to get an update. And password prompts were happening.

Comment 44

[Wayne Mery (:wsmwk)]

For 115.0, this is #1 crash. For example Bp-4a193911-3e27-4bd5-8f07-a8b360230714

Comment 45

[Makoto Kato [:m_kato]]

Gecko thread is going to shut down. But IMAP thread doesn't finish since OAuth2's monitor isn't signal.

• nsImapProtocol::AuthLogin should return error immediately if shutdown phase is AppShutdown or something.
• Use RunOnShutdown or shutdown observer to signal OAuth2's monitor.
• (optional) GetXOAuth2String and SupportsOAuth2 should check return value of NS_DispatchToMainThread. It can return error. If error, this lock occurs.

Comment 46

[Arthur K. (he/him)]

This is activity after removing all saved PWs: https://share.firefox.dev/458JLWb
(In reply to Wayne Mery (:wsmwk) from comment #43)

(In reply to Arthur K. (he/him) from comment #42)

So here's another possible clue it might be password manager or OAuth2 related.

Very possible.

I just crashed nightly bp-900ad218-a365-4f1b-b64a-2724e0230707 during a short startup that was needed to get an update. And password prompts were happening.

I tested this with 115.1.0 RC and it still crashed but took well close to 5 minutes before crash handler showed up: https://crash-stats.mozilla.org/report/index/a42f3508-e568-4764-bf68-49a600230730

Here's hoping bug 1788599 will help out here. If we do spin a 115.1.1 with bug 1788599 in it, I'll try and remember to come back here and test again.

Comment 47

[Wayne Mery (:wsmwk)]

(In reply to Arthur K. (he/him) from comment #46)

Here's hoping bug 1788599 will help out here. If we do spin a 115.1.1 with bug 1788599 in it, I'll try and remember to come back here and test again.

1788599 may help with the amount of time. But I would not expect it to help with the crash.

Comment 48

[Kai Engert [:KaiE:]]

Arthur, do you use a smartcard?

Comment 49

[Kai Engert [:KaiE:]]

(I wonder if Thunderbird has a third party pkcs#11 module loaded, which might not release some resources on shutdown, this idea is just a shot in the wild.)

Comment 50

[Arthur K. (he/him)]

(In reply to Kai Engert (:KaiE:) from comment #48)

Arthur, do you use a smartcard?

I do not.

Comment 51

[Wayne Mery (:wsmwk)]

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

(In reply to Arthur K. (he/him) from comment #46)

Here's hoping bug 1788599 will help out here. If we do spin a 115.1.1 with bug 1788599 in it, I'll try and remember to come back here and test again.

1788599 (115.1.0) may help with the amount of time. But I would not expect it to help with the crash.

Better?

Comment 52

[Arthur K. (he/him)]

Nope. And when it crashes I can't generate a Crash Report. This was on Portable 115.3.1. Waiting on OWL plugin to be updated against 115.3.2 (as it currently doesn't work).

Comment 53

[Arthur K. (he/him)]

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

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

(In reply to Arthur K. (he/him) from comment #46)

Here's hoping bug 1788599 will help out here. If we do spin a 115.1.1 with bug 1788599 in it, I'll try and remember to come back here and test again.

1788599 (115.1.0) may help with the amount of time. But I would not expect it to help with the crash.

Better?

I was able to cause a crash and get a crash report using 115.4.1: https://crash-stats.mozilla.org/report/index/d5c2439d-a6f3-4541-ab87-ce9f30231026

Comment 54

[Wayne Mery (:wsmwk)]

Attachment: Screenshot 2023-10-26 at 10.51.24 AM.png

I was able to cause a crash and get a crash report using 115.4.1: https://crash-stats.mozilla.org/report/index/d5c2439d-a6f3-4541-ab87-ce9f30231026

definitely matches to this bug.

Crash rate for this signature is significantly higher for version 115.

But there may be other reasons for it:

• bp-72a70ee3-a328-46fe-a4d5-0e3bb0231024 claims to have blown away the profile
• bp-0a282cf8-0795-4225-bee9-7ccf80231023 states " crashes everyday on me."
• bp-e98e046c-fe47-4f6a-8cff-8495e0231023. Thunderbird will not hold the setting up of passwords with microsoft's outlook.com and hotmail.com email addreses. I am almost to the point to uninstall thunderbird completely because of this.
• bp-b26d5e82-9939-458c-9036-5193d0231020 Opened Thunderbird, soon closed Thunderbird.
• bp-3a63dff6-ab61-4e32-8971-9b0e00231019 I was trying to enter an appointment in the Calendar, but it would not take the modification I entered. Then the program crashed.
• bp-631a5444-2dbe-445f-bacd-5fc800231005 I am getting time and again being forced to keep putting my passwords to my Microsoft account

Comment 55

[Arthur K. (he/him)]

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

Created attachment 9360437 [details]
Screenshot 2023-10-26 at 10.51.24 AM.png

I was able to cause a crash and get a crash report using 115.4.1: https://crash-stats.mozilla.org/report/index/d5c2439d-a6f3-4541-ab87-ce9f30231026

definitely matches to this bug.

Crash rate for this signature is significantly higher for version 115.

If not matching, it's in the same orbit or a correlary of bug 1505660 I would think. We shou'dn't go so far as to say a dupe as my STR are at least repeatable but, oddly, only by me thus far. =\

Is it worth it at all to get another perf profile as TB is shutting down after I do my STR? I can if you guys think it will help.

Comment 56

[Wayne Mery (:wsmwk)]

Crash rate for this signature is significantly higher for version 115.

100-fold increase - see https://bugzilla.mozilla.org/attachment.cgi?id=9360437

Other cash signatures mentioning password;

• bp-3e9cd7fb-4cf7-40af-9ee1-1260d0231026 mozilla::dom::workerinternals::RuntimeService::CrashIfHanging
• bp-70cac9ad-5c5d-4526-b489-dba560231025 shutdownhang | mozilla::SpinEventLoopUntil<T> | nsThread::Shutdown | nsThreadManager::ShutdownNonMainThreads
• bp-2352ed51-26d9-40d3-868f-dd79b0230914 xul.dll | _InternalCallWinProc
• bp-2bac9af2-96d0-4cbb-b86d-01b660230902 NSSSocketControl::StartTLS | NS_InvokeByIndex

Comment 57

[Jens Stutte [:jstutte]]

I was able to cause a crash and get a crash report using 115.4.1: https://crash-stats.mozilla.org/report/index/d5c2439d-a6f3-4541-ab87-ce9f30231026

seems to be the same as comment 45.

If not matching, for it's in the same orbit or a correlary of bug 1505660 I would think. We shou'dn't go so far as to say a dupe as my STR are at least repeatable but, oddly, only by me thus far.

Bug 1505660 wants to look at Firefox while this bug wants to examine the Thunderbird specific variants, so no, we should not dupe them and your STR and any additional information fit here , not there, thanks.

Is it worth it at all to get another perf profile as TB is shutting down after I do my STR? I can if you guys think it will help.

I think comment 45 already says it all - the Thunderbird code is lacking a shutdown observer or blocker that ensures this wait is interrupted. From what I see in crash-stats I do not think that a new profile will add much information here, thanks.

Comment 58

[Wayne Mery (:wsmwk)]

I think comment 45 already says it all - the Thunderbird code is lacking a shutdown observer or blocker that ensures this wait is interrupted. From what I see in crash-stats I do not think that a new profile will add much information here, thanks.

Correct. And, we know how to reproduce.

Crash rate for this signature is significantly higher for version 115.

Thunderbird is almost at 2,000 crashes per day

Comment 59

[Magnus Melin [:mkmelin]]

The steps to reproduce do not hang for me on linux. But I'll take on addressing comment 45.

Comment 60

[Magnus Melin [:mkmelin]]

Attachment: Bug 1768344 - nsImapProtocol::AuthLogin should bail if we're shutting down. r=babolivier,benc

Comment 61

[Magnus Melin [:mkmelin]]

Attachment: Bug 1768344 - Use RunOnShutdown or shutdown observer to signal OAuth2's monitor. r=babolivier,benc

Depends on D192569

Comment 62

[Magnus Melin [:mkmelin]]

Attachment: Bug 1768344 - GetXOAuth2String and SupportsOAuth2 should check return value of NS_DispatchToMainThread. r=babolivier,benc

Depends on D192570

Comment 63

[Arthur K. (he/him)]

Will these get backported to ESR?

Comment 64

[Magnus Melin [:mkmelin]]

Sure, but first reviews, landing on trunk and beta.

Comment 65

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/68ee42fc2136
nsImapProtocol::AuthLogin should bail if we're shutting down. r=BenC
https://hg.mozilla.org/comm-central/rev/b36435b2d46f
Use RunOnShutdown or shutdown observer to signal OAuth2's monitor. r=BenC
https://hg.mozilla.org/comm-central/rev/72918f80c378
GetXOAuth2String and SupportsOAuth2 should check return value of NS_DispatchToMainThread. r=BenC

Comment 66

[Magnus Melin [:mkmelin]]

Comment on attachment 9361601 [details]
Bug 1768344 - GetXOAuth2String and SupportsOAuth2 should check return value of NS_DispatchToMainThread. r=babolivier,benc

[Approval Request Comment]
User impact if declined: may crash
Testing completed (on c-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): Unclear. Should bake on data beta and see how it goes.

Comment 67

[Wayne Mery (:wsmwk)]

Comment on attachment 9361601 [details]
Bug 1768344 - GetXOAuth2String and SupportsOAuth2 should check return value of NS_DispatchToMainThread. r=babolivier,benc

[Triage Comment]
Approved for beta

Comment 68

[Magnus Melin [:mkmelin]]

Take all three patches, if it wasn't clear.

Comment 69

[Rob Lemley [:rjl]]

Thunderbird 120.0b5:
https://hg.mozilla.org/releases/comm-beta/rev/25660f7dbfb0
https://hg.mozilla.org/releases/comm-beta/rev/94d12a8d6392
https://hg.mozilla.org/releases/comm-beta/rev/e0e294c5ca2f

Comment 70

[Wayne Mery (:wsmwk)]

Odd - there are no crashes for signatures shutdownhang | mozilla::SpinEventLoopUntil | nsThread::Shutdown | nsThreadManager::ShutdownNonMainThreads since 120.0b1*, and 102.0b1 predates fixing of this bug on beta. So what happened to the signature?

• https://crash-stats.mozilla.org/signature/?release_channel=%21release&product=Thunderbird&signature=shutdownhang%20%7C%20mozilla%3A%3ASpinEventLoopUntil%20%7C%20nsThread%3A%3AShutdown%20%7C%20nsThreadManager%3A%3AShutdownNonMainThreads&date=%3E%3D2023-10-27T13%3A35%3A00.000Z&date=%3C2023-11-10T13%3A35%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_columns=startup_crash&_sort=-date&page=1#summary

Should be safe but I feel like this should have a full week of beta, so declining to take for 115.4.3.

Comment 71

[Wayne Mery (:wsmwk)]

Essentially the crash rate for beta dropped by half on Friday Oct 27. I can't explain it.

Comment 72

[Wayne Mery (:wsmwk)]

Arthur, does the crash stop with 120.0b5?

Comment 73

[Arthur K. (he/him)]

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

Arthur, does the crash stop with 120.0b5?

Yup. Wow, that's a huge win. Did Magnus' patches not make it into 120b5 yet but it's magically "fixed"?

Comment 74

[Wayne Mery (:wsmwk)]

See comment 59, correction comment 69, for the patch landing in 120.0b5.

Comment 75

[gene smith]

Earlier today I updated to c-c tip and I now crash when I open any folder of any imap account using oauth2. It's fine when just using normal passwords.
I was able to bisect and see that everything earlier than changeset 40176 doesn't crash on oauth2 accounts. From my hg log -G:

:
o  changeset:   40177:72918f80c378 (crashes)
|  user:        Magnus Melin <[e-mail]>
|  date:        Mon Nov 06 09:24:37 2023 +0200
|  summary:     Bug 1768344 - GetXOAuth2String and SupportsOAuth2 should check return value of NS_DispatchToMainThread. r=BenC
|
o  changeset:   40176:b36435b2d46f  (crashes)
|  user:        Magnus Melin <[e-mail]>
|  date:        Mon Nov 06 09:24:55 2023 +0200
|  summary:     Bug 1768344 - Use RunOnShutdown or shutdown observer to signal OAuth2's monitor. r=BenC
|
@  changeset:   40175:68ee42fc2136 (First one that doesn't crash on oauth2 acc't access)
|  user:        Magnus Melin <[e-mail]>
|  date:        Mon Nov 06 09:25:06 2023 +0200
|  summary:     Bug 1768344 - nsImapProtocol::AuthLogin should bail if we're shutting down. r=BenC
:

Currently 40175 is checked out and it's not crashing. 40176 and higher always crash on oauth2 login attempts.
Since I have many test accounts in my profile, I always run with "Check for new mail at startup" not selected so I have to manually click a folder to trigger a imap connection and authentication.
Note: There's also a strange delays on initial startup (content of my IMAP:4 log has 20 second or so time with no activity) with c-c tip but I'm not seeing it with above 40175 checked out. I don't think it's related to to this but it might be.

Comment 76

[Ben Campbell]

Potential problem with:

https://hg.mozilla.org/comm-central/rev/b36435b2d46f

In matrix tb-developers, Itagagaki reported problems:

Assertion failure: !mRawPtr, at C:/Users/ita/source/repos/Mozilla/obj-x86_64-pc-windows-msvc/dist/include/mozilla/AlreadyAddRefed.h:136
#01: mozilla::mailnews::OAuth2ThreadHelper::GetXOAuth2String (C:\Users\ita\source\repos\Mozilla\comm\mailnews\imap\src\nsSyncRunnableHelpers.cpp:522)

I think the fix is that the capture clause for the inner lambda should be [=] instead of [&]. The inner lambda needs to hold its own copy of the self pointer object, not just a reference to it.
But now I'm also wondering what the code does - isn't NS_NewRunnableFunction() meant to return a Runnable? But in that changeset, any return value is just discarded... or am I missing something?

Obviously whoever reviewed that patch did a very shoddy job. cough

Comment 77

[gene smith]

Attachment: oauth2-crash-fix.diff

Not really a "fix" but this diff gets rid of (for me) the crash on access of oauth2 account (e.g., gmail, yahoo). I have no idea why this causes a crash for me.
Of the 3 diffs that were committed for this bug, only the one that changed nsImapProtocol.cpp, changeset: 40175:68ee42fc2136, seems to not cause a problem.
Edit: Didn't see Ben's comment above before writing this. Looks like he may have a handle on the problem.

Comment 78

[Magnus Melin [:mkmelin]]

Yet the patch wasn't quite right. Fixing it in bug 1864747.

Comment 79

[Wayne Mery (:wsmwk)]

I guess we will want bug 1864747 to go through beta before taking this all on version 115.

Comment 80

[Wayne Mery (:wsmwk)]

Beta crashes end after 119.0b6 (at statistically significant rates), and the last nightly crash was 118.0a1 (not statistically significant).
So v.fixed for 121 and 120.

Comment 81

[Wayne Mery (:wsmwk)]

Comment on attachment 9361598 [details]
Bug 1768344 - nsImapProtocol::AuthLogin should bail if we're shutting down. r=babolivier,benc

[Triage Comment]
Approved for esr115

Comment 82

[Wayne Mery (:wsmwk)]

Comment on attachment 9361600 [details]
Bug 1768344 - Use RunOnShutdown or shutdown observer to signal OAuth2's monitor. r=babolivier,benc

[Triage Comment]
Approved for esr115

Comment 83

[Wayne Mery (:wsmwk)]

Comment on attachment 9361601 [details]
Bug 1768344 - GetXOAuth2String and SupportsOAuth2 should check return value of NS_DispatchToMainThread. r=babolivier,benc

[Triage Comment]
Approved for esr115

Comment 84

[Rob Lemley [:rjl]]

Thunderbird 115.5.2:
https://hg.mozilla.org/releases/comm-esr115/rev/4f382be54b08
https://hg.mozilla.org/releases/comm-esr115/rev/260f1d718265

Comment 85

[Wayne Mery (:wsmwk)]

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

Beta crashes end after 119.0b6 (at statistically significant rates), and the last nightly crash was 118.0a1 (not statistically significant).
So v.fixed for 121 and 120.

But release crashes continue - still happening for 115.5.2 https://crash-stats.mozilla.org/signature/?signature=shutdownhang%20%7C%20mozilla%3A%3ASpinEventLoopUntil%20%7C%20nsThread%3A%3AShutdown%20%7C%20nsThreadManager%3A%3AShutdownNonMainThreads#summary

Comment 86

[Wayne Mery (:wsmwk)]

I forgot we have bug 1869297

Comment 87

[Arthur K. (he/him)]

Not assuming anything has radically changed since I opened this bug but do we need to edit the title from:

@ shutdownhang | kernelbase.dll | mozilla::TaskControlleMr::GetRunnableForMTTask | nsThread::Shutdown | nsThreadManager::ShutdownNonMainThreads ] mozilla::SpinEventLoopUntil

to just

@ shutdownhang | mozilla::SpinEventLoopUntil | nsThread::Shutdown | nsThreadManager::ShutdownNonMainThreads ]

since this is more accurate per 115.5.2 and less "kitchen sink-y" from when I/we bolted on kernelbase.dll and mozilla::TaskControlleMr::GetRunnableForMTTask in the title?

https://crash-stats.mozilla.org/report/index/af232964-e76c-4a72-985f-4bc870231214

Comment 88

[gene smith]

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

Comment on attachment 9361598 [details]
Bug 1768344 - nsImapProtocol::AuthLogin should bail if we're shutting down. r=babolivier,benc

[Triage Comment]
Approved for esr115

This patch never got put into esr115 from what I can tell. It is in daily/c-c (haven't checked beta) that I've been running with and it explains why when I shutdown with "empty trash on exit" or "expunge inbox on exit" nothing happens. (I had thought it was a timing or maybe a win vs. linux issue.) ESR115 works correctly when I shutdown since it doesn't yet have this patch. I think the patch needs to still allow some URLs to run (deleteallmsg, expunge inbox, etc) when shutting down.
The change I'm working on to fix the 115.10.0 shutdown hang includes this patch but modified to still allow the URLs to run that occur at shutdown and are needed by the "clean up on exit" features in imap server settings.

Comment 89

[Wayne Mery (:wsmwk)]

(In reply to gene smith from comment #88)

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

Comment on attachment 9361598 [details]
Bug 1768344 - nsImapProtocol::AuthLogin should bail if we're shutting down. r=babolivier,benc

[Triage Comment]
Approved for esr115

This patch never got put into esr115 from what I can tell. It is in daily/c-c (haven't checked beta) that I've been running with and it explains why when I shutdown with "empty trash on exit" or "expunge inbox on exit" nothing happens. (I had thought it was a timing or maybe a win vs. linux issue.) ESR115 works correctly when I shutdown since it doesn't yet have this patch. I think the patch needs to still allow some URLs to run (deleteallmsg, expunge inbox, etc) when shutting down.
The change I'm working on to fix the 115.10.0 shutdown hang includes this patch but modified to still allow the URLs to run that occur at shutdown and are needed by the "clean up on exit" features in imap server settings.

So bug 1896545. Might this also help with master password issues?

Comment 90

[gene smith]

Not sure what the master password issues are. I thought it was something to do with OSX?
Bug 1896545 suggests a way to allow some URLs to occur at shutdown instead of forbidding them all as patch "attachment 9361598 [details]" does. That patch was never applied to 115 which is good (but don't know if it was intentional).