Bugzilla

Comment 1

•

4 years ago

? Related to Bug 1175168 - crash nsXPCWrappedJS::CallMethod - need to proxy URL release (JavaScript's XPCOM object) to the main thread in the IMAP protocol (Not import related)

Keywords: crash

Updated

•

4 years ago

Version: unspecified → Thunderbird 82

Comment 2

•

3 years ago

This bug has been affecting me for several months, i'm using thunderbird 84 on windows.

This crashes occur very frequently when the session is reopening multiple pdf documents of the previous session. Deleting session.json is a manual workaround to get out of the restarting-crash-restarting-crash-... loop

PDF attachment viewing is a great improvement to Thunderbird, but it comes with this recurring downside...

Comment 3

•

3 years ago

(In reply to germinolegrand from comment #2)

This bug has been affecting me for several months, i'm using thunderbird 84 on windows.

What are your crash IDs?

Flags: needinfo?(germinolegrand)

Comment 4

•

3 years ago

The crash rate for betas greatly increased with 84, and and is still high with 85.
https://crash-stats.mozilla.org/signature/?release_channel=%21release&signature=nsXPCWrappedJS%3A%3ACallMethod&date=%3E%3D2020-11-19T03%3A18%3A00.000Z&date=%3C2021-01-19T03%3A18%3A00.000Z#summary

Comment 5

•

3 years ago

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

(In reply to germinolegrand from comment #2)

This bug has been affecting me for several months, i'm using thunderbird 84 on windows.

What are your crash IDs?

Here they are :

Flags: needinfo?(germinolegrand)

Reporter

Comment 6

•

3 years ago

This bug is at least somewhat reproducible. Unless there are other ideas, maybe we shouldn't restore pdf tabs, and thus avoiding the problem.

Priority: -- → P2

Mark Banner (:standard8)

Comment 7

•

3 years ago

From the looks of the stacks, the assertions are happening on the DOM worker thread. I don't know exactly what that is, but here's what I guess is happening:

This is mainly based on bp-bee0c084-bfc6-45bf-b823-587740200909

Thunderbird starts up, IMAP service hasn't necessarily kicked in yet.
A tab with an attachment is restored, which has an imap:// url. For some reason, this happens on the DOM worker thread.
That calls into NS_NewMailnewsURI which then calls nsImapService::NewURI: https://searchfox.org/comm-central/rev/bd4526e24041709d3b988ecaca782e4a19982dd9/mailnews/base/src/nsNewMailnewsURI.cpp#25-27,45
We then get through to nsImapService::GetServerFromUrl -> nsMsgAccountManager::FindServerByURI -> nsMsgAccountManager::findServerInternal

At that stage, we're probably trying to access something from javascript, but because we're in the DOM worker thread, we fail.

Interestingly, later in NS_NewMailnewsURI, we do some main thread checking and throw across to the main thread if necessary: https://searchfox.org/comm-central/rev/bd4526e24041709d3b988ecaca782e4a19982dd9/mailnews/base/src/nsNewMailnewsURI.cpp#111-117

Maybe something like that would work for the whole call?

It might be worth checking my analysis with someone from the gecko team, they might understand why this is happening on the DOM worker thread, and the correct way to handle this.

Updated

•

3 years ago

Crash Signature: [@ nsXPCWrappedJS::CallMethod ]

Summary: crash [@ nsXPCWrappedJS::CallMethod ] on startup restoring tab with pdf attachment from imap → Thuderbird crash [@ nsXPCWrappedJS::CallMethod ] on startup restoring tab with pdf attachment from imap

Comment 8

•

3 years ago

Hello, now i get this crash absolutely every time i open a pdf attachment since 89.0b3 or 89.0b4. Still here in 90.0b1. Very easy to reproduce on my side, but i can't open any pdf within thunderbird...

Before you ask :
https://crash-stats.mozilla.org/report/index/e4852fe1-9600-4e99-afdc-492c80210604
https://crash-stats.mozilla.org/report/index/fd6e33ed-d852-41da-8c02-d52ec0210604

Comment 9

•

3 years ago

(In reply to germinolegrand from comment #8)

Hello, now i get this crash absolutely every time i open a pdf attachment since 89.0b3 or 89.0b4. Still here in 90.0b1. Very easy to reproduce on my side, but i can't open any pdf within thunderbird...

Reporter

Comment 10

•

3 years ago

Yeah I think this bug morphed, and it's no longer only for restore. Basically all pdf opening crash for me nowadays.

MOZ_RELEASE_ASSERT(NS_IsMainThread()) (nsXPCWrappedJS::CallMethod called off main thread)

tracking-thunderbird91: --- → +

Priority: P2 → P1

Comment 11

•

3 years ago

#2 crash for beta 90. Looks like it first became a topcrash in beta 89, at end of April, with buildid 20210419235748 - https://crash-stats.mozilla.org/signature/?release_channel=beta&product=Thunderbird&signature=nsXPCWrappedJS%3A%3ACallMethod&date=%3E%3D2021-03-10T13%3A35%3A00.000Z&date=%3C2021-06-10T13%3A35%3A00.000Z#summary

In nightlies 90.0a1, I would say it started around 20210405104606 - https://crash-stats.mozilla.org/signature/?release_channel=nightly&product=Thunderbird&signature=nsXPCWrappedJS%3A%3ACallMethod&date=%3E%3D2021-03-10T13%3A35%3A00.000Z&date=%3C2021-06-10T13%3A35%3A00.000Z#aggregations

Keywords: topcrash-thunderbird

Comment 12

•

3 years ago

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

Basically all pdf opening crash for me nowadays.

Is it as simple as opening any/every old pdf? Or are steps to reproduce slightly more complicated?

I don't crash when opening a pdf attachment on Mac.* I wonder why.

But I did just crash - bug 1674954 comment 39 - and a pdf had been open for several days

Flags: needinfo?(mkmelin+mozilla)

Comment 13

•

3 years ago

(In reply to Mark Banner (:standard8) from comment #7)

...
It might be worth checking my analysis with someone from the gecko team, they might understand why this is happening on the DOM worker thread, and the correct way to handle this.

Who is the person to ask?

Comment 14

•

3 years ago

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

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

Basically all pdf opening crash for me nowadays.

Is it as simple as opening any/every old pdf? Or are steps to reproduce slightly more complicated?

I don't crash when opening a pdf attachment on Mac.* I wonder why.

But I did just crash - bug 1674954 comment 39 - and a pdf had been open for several days

Any pdf of any sort for me on windows. Just opening it. Always.

Reporter

Comment 15

•

3 years ago

The STR are to open any PDF that's attached to an email on IMAP. Doesn't crash if it's from a local folder.

Flags: needinfo?(mkmelin+mozilla)

Reporter

Comment 16

•

3 years ago

Attached file attached pdf.eml — Details

Simple test case. Copy to IMAP, try to open the PDF. Crash.

Reporter

Comment 17

•

3 years ago

Possibly what we should do is for IMAP and maybe NNTP, get the messages and put it into a data URI before passing it off to get opened.
fetch() on the imap url could work.

Code should be around here: https://searchfox.org/comm-central/rev/b972d9ae1bd20536044be70c5bf3670e70b40720/mail/base/content/msgHdrView.js#1931

Reporter

Comment 18

•

3 years ago

•

Edited

Attached file WIP: bug1663859_pdf_dataurl.patch — Details

WIP - the basic idea seems ok (pdfs do open for local messages), but needs more work.

Reporter

Comment 19

•

3 years ago

Lasana, can you take it from here?

Assignee: nobody → lasana

Mark Banner (:standard8)

Comment 20

•

3 years ago

Might this cause an issue with our session restore code, especially for large PDFs? I'm guessing that would require saving the full data url in the restore file, which would then need to be loaded on startup potentially causing performance issues if we're doing it in a blocking fashion.

Could the real issue be that IMAP hasn't fully started by the time we're trying to restore the tab, and we should wait on something before we do that?

Reporter

Comment 21

•

3 years ago

It used to crash for session restore, but it's mutated to being always for imap. (Updating the summary now.)

Session restore for a data: url could be somewhat problematic. OTOH, it's probably loading faster in total than trying to restore a large PDF over IMAP.

Summary: Thuderbird crash [@ nsXPCWrappedJS::CallMethod ] on startup restoring tab with pdf attachment from imap → Thuderbird crash [@ nsXPCWrappedJS::CallMethod ] when opening pdf attachment from imap

Assignee

Comment 22

•

3 years ago

•

Edited

Was this fixed already? I can open pdf files without crashing. Or is it not happening on Linux?

Flags: needinfo?(mkmelin+mozilla)

Reporter

Comment 23

•

3 years ago

I tried it earlier today on linux, with the test case above copied to a gmail account on imap. Crashed instantly. Of course pdfjs needs to be enabled (which it is by default).

Flags: needinfo?(mkmelin+mozilla)

Landry Breuil (:gaston)

Comment 24

•

3 years ago

Comment on attachment 9226773 [details]
WIP: bug1663859_pdf_dataurl.patch

Fwiw, i'm seeing the crash too on OpenBSD with 90.b1 (and since some betas) when opening pdfs attached to mails.

With this patch it no longer crashes but doesnt open the file, and i have this in the error console:

Uncaught (in promise) TypeError: Window.fetch: imap://user@server:143/fetch%3EUID%3E/INBOX%3E36273?part=1.2&filename=Acc%C3%A8s%20T%C3%A9l%C3%A9com%20CRAIG%20V1.2%20%28TVA%29.pdf is an url with embedded credentials.
    open chrome://messenger/content/msgHdrView.js:1942
msgHdrView.js:1942:23

Reporter

Comment 25

•

3 years ago

•

Edited

Yep it's wip and not expected to work for imap messages.

Assignee

Comment 26

•

3 years ago

I still can't reproduce this, I tried with a debug build, with some of the extensions I saw installed but to no avail yet.

Comment 27

•

3 years ago

•

Edited

We also see PDF crashes, and from what I understood: The PDF renderer should be running in a content process (not IMAP thread, and not main thread, not main process), but for that to work, the data for the URL needs to be send over the process boundary.

Zoe Martin

Comment 28

•

3 years ago

After some debugging we followed Mark's analysis from comment #7 and came up with this. It fixes the crash:
https://github.com/Betterbird/thunderbird-patches/blob/main/91/bugs/1663859-imap-uri-main-tread-crash-fix.patch

Assignee

Comment 29

•

3 years ago

Great! I'm changing the assignee since you already got something.

Assignee: lasana → zoe

Zoe Martin

Comment 30

•

3 years ago

Thanks for the honour, but there seems to be a misunderstanding. Betterbird is a Thunderbird fork and we're giving back to upstream (as any good downstream project should do). We're not planning to be involved in your project above that (and we'd also like to keep comments to a minimum, ideally one per bug, so it doesn't look like we'd be advertising our stuff). If you like the patch, please take it, and we'd appreciate if you preserved the authorship like we have acknowledged Mark's analysis/idea in the commit message.

Assignee: zoe → nobody

Comment 31

•

3 years ago

•

Edited

Thanks, Zoe! This is the biggest TB 91 blocker right now. I appreciate you looking into it and fixing it.

The fix seems to be the same code as Neil did for Owl in bug 1555633 at the end of the same file.

Minor code NITs (doesn't change functionality at all, just easier to read code):

Either move the inline function into the else block, where it's used, or use the function NewURI also in the if block, like: NewURI(); return rv;

Comment 32

•

3 years ago

•

Edited

@Lasana: Can you take this bug back, please?

Severity: -- → S2

Flags: needinfo?(lasana)

Summary: Thuderbird crash [@ nsXPCWrappedJS::CallMethod ] when opening pdf attachment from imap → Crash when opening an PDF attachment from IMAP [@ nsXPCWrappedJS::CallMethod ]

Zoe Martin

Comment 33

•

3 years ago

Minor code NITs ...

Copied what was there (wondering why it was done like that and marvelling at the C++ syntax).

Assignee

Comment 34

•

3 years ago

(In reply to Zoe Martin from comment #30)

Thanks for the honour, but there seems to be a misunderstanding. Betterbird is a Thunderbird fork and we're giving back to upstream (as any good downstream project should do). We're not planning to be involved in your project above that ...

Thanks for letting me know.

@Lasana: Can you take this bug back, please?

I have not been able to reproduce this bug for some reason so I can't test if the patch works or not. My cpp is still basic so I really can't tell if it's doing the right thing or not by just looking.

Flags: needinfo?(lasana)

Landry Breuil (:gaston)

Comment 35

•

3 years ago

I'm not 100% sure my testing is right, but porting https://github.com/Betterbird/thunderbird-patches/blob/main/91/bugs/1663859-imap-uri-main-tread-crash-fix.patch on top of 90.0b3 self built on OpenBSD allows me to open PDF attachments in a thunderbird tab, from a mail on an IMAP connection (eg the tab URL is imap://landry@localhost:1143/fetch%3EUID%3E.INBOX%3E27962?part=1.2&filename=Proc%C3%A9dure%20d%27inscription.pdf&type=application/pdf). That's not on the computer when i was seeing thunderbird crashing (with 90.0b3 unpatched) when opening PDF attachments, where i'll be able to test monday.

Updated

•

3 years ago

status-thunderbird91: --- → affected

OS: Unspecified → All

Landry Breuil (:gaston)

Comment 36

•

3 years ago

tested with 90.0b3 on my work laptop (Also OpenBSD) where i've had crashes opening PDFs since some betas, can now definitely open PDF attachments from mails stored on IMAP. Definitely worth improvement, especially if that's a blocker for 91 !

Assignee

Comment 37

•

3 years ago

Attached file Bug 1663859 - Move all IMAP URI creation onto the main thread. r=mkmelin — Details

Phabricator Automation

Updated

•

3 years ago

Assignee: nobody → lasana

Status: NEW → ASSIGNED

Phabricator Automation

Updated

•

3 years ago

Attachment #9230776 - Attachment description: Bug 1663859 - Move all IMAP URI creation onto the main thread (analysis by Mark Banner). r=mkmelin → Bug 1663859 - Move all IMAP URI creation onto the main thread. r=mkmelin

Assignee

Updated

•

3 years ago

Keywords: checkin-needed-tb

Target Milestone: --- → 92 Branch

Thomas D. (:thomas8)

Updated

•

3 years ago

Comment 38

•

3 years ago

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/76ece33df0c5
Move all IMAP URI creation onto the main thread. r=mkmelin

Status: ASSIGNED → RESOLVED

Closed: 3 years ago

Keywords: checkin-needed-tb

Resolution: --- → FIXED

Comment 39

•

3 years ago

Thank you!

Pulsebot

Comment 40

•

3 years ago

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/0ae34b27f11a
follow-up - Add a missing line to unbreak the build. rs=bustage-fix