1766169 - Crash in [@ memset | nsXPCWrappedJS::CleanupOutparams]

Status: VERIFIED FIXED

(MailNews Core :: Networking: POP, defect, P1)

Description

[Wayne Mery (:wsmwk)]

Starting with beta 99[1], The crash signature of Bug #1696893 returns, but the stack is different.

Maybe Fission related. (DOMFissionEnabled=1)

Crash report: https://crash-stats.mozilla.org/report/index/fc15e988-f2f8-465b-a1b2-396b20220423

Reason: EXCEPTION_ACCESS_VIOLATION_WRITE

Top 5 frames of crashing thread:

0 vcruntime140.dll memset d:\agent\_work\2\s\src\vctools\crt\vcruntime\src\string\i386\memset.asm:173
1 xul.dll static nsXPCWrappedJS::CleanupOutparams js/xpconnect/src/XPCWrappedJSClass.cpp:591
2 xul.dll nsXPCWrappedJS::CallMethod js/xpconnect/src/XPCWrappedJSClass.cpp:962
3 xul.dll PrepareAndDispatch xpcom/reflect/xptcall/md/win32/xptcstubs.cpp:79
4 xul.dll SharedStub xpcom/reflect/xptcall/md/win32/xptcstubs.cpp:98

[1] 99 beta bp-ea1ed557-890d-49fd-aaa5-e90e80220407 buildid 20220321154912

Comment 1

[Wayne Mery (:wsmwk)]

#3 crash for beta

Comment 2

[Magnus Melin [:mkmelin]]

Why do you think it's related to nsMsgMdnGenerator? (We could potentially rewrite that in js though.)
Without more information, we don't have a lot to go on. Are we sure it started with 99?

Comment 3

[Wayne Mery (:wsmwk)]

I don't think it is related - I forgot to take nsMsgMdnGenerator off of the summary.

Comment 4

[Wayne Mery (:wsmwk)]

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

Without more information, we don't have a lot to go on. Are we sure it started with 99?

It can't be stated with 100% certainty that it started with 99, because we only have 6 months of crash data, and no crash-stats.mozilla.org for Nov-Feb. But crash-stats indicates it may have started with 99, per https://crash-stats.mozilla.org/signature/?signature=memset%20%7C%20nsXPCWrappedJS%3A%3ACleanupOutparams&date=%3E%3D2021-10-25T06%3A40%3A00.000Z&date=%3C2022-04-25T06%3A40%3A00.000Z&_sort=-date
Sorry don't have more solid info

Comment 5

[Wayne Mery (:wsmwk)]

memset | nsXPTType::ZeroValue may be the same issue. But it first appears with 101.0b1, not beta 99.
bp-471d67ab-a620-4259-b5fd-4644d0220507
0 vcruntime140.dll memset(unsigned char*, unsigned char, unsigned long) d:\agent_work\1\s\src\vctools\crt\vcruntime\src\string\i386\memset.asm:173
1 xul.dll nsXPTType::ZeroValue(void*) const xpcom/reflect/xptinfo/xptinfo.h:288
2 xul.dll static nsXPCWrappedJS::CleanupOutparams(nsXPTMethodInfo const*, nsXPTCMiniVariant*, bool, unsigned char) js/xpconnect/src/XPCWrappedJSClass.cpp:591
3 xul.dll nsXPCWrappedJS::CallMethod(unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:962
4 xul.dll PrepareAndDispatch(nsXPTCStubBase*, unsigned int, unsigned int*, unsigned int*) xpcom/reflect/xptcall/md/win32/xptcstubs.cpp:79
5 xul.dll SharedStub() xpcom/reflect/xptcall/md/win32/xptcstubs.cpp:98

Comment 6

[Wayne Mery (:wsmwk)]

still a strong topcrash for beta

Comment 7

[Wayne Mery (:wsmwk)]

bp-57ecf5a3-3f4e-49dd-83c9-029f90220619 tb102

Comment 8

[Wayne Mery (:wsmwk)]

#2 crash for version 102. Any ideas?

Comment 9

[Makoto Kato [:m_kato]]

This is regression from bug 1738173 that c-c moves POP3 implementation to JSM.

I guess that this is similar to bug 1751576. We shouldn't pass nullptr for out parameter.

Comment 10

[Wayne Mery (:wsmwk)]

Thanks for that analysis.

Comment 11

[Ping Chen [:rnons] (away)]

Attachment: Bug 1766169 - Fix crashing by removing unused return value of nsILocalMailIncomingServer.getNewMail. r=benc

Crash happens because nullptr is used to receive return value of a JS xpcom function.

Comment 12

[Ping Chen [:rnons] (away)]

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

bp-57ecf5a3-3f4e-49dd-83c9-029f90220619 tb102

This has the most useful backtrace.

To reproduce, click the bottom-left icon to go offline, a dialog is shown asking whether to download, click the Download button, crash.
I don't know why nsImapOfflineSync.cpp calls nsLocalMailFolder.cpp which then calls Pop3IncomingServer.jsm.

Comment 13

[Wayne Mery (:wsmwk)]

If you have confidence in this patch, it would be great to have it land on Friday morning's nightly, and then Friday's beta. Please request uplift with your assessment of risk.

This would seem to be a regression - and it would be nice to know the cause - but more important that we understand and have confidence in the fix.

Comment 14

[Kai Engert (:KaiE:)]

I'll attach an alternative patch.

Comment 15

[Kai Engert (:KaiE:)]

Attachment: Bug 1766169 - Ensure all callers of Pop3/GetNewMail can correctly receive the result from the JS XPCOM implementation. r=rnons.

Thanks to Ping Chen (:rnons) for the initial analysis.

Comment 16

[Kai Engert (:KaiE:)]

Comment on attachment 9284182 [details]
Bug 1766169 - Ensure all callers of Pop3/GetNewMail can correctly receive the result from the JS XPCOM implementation. r=rnons.

[Approval Request Comment]
Regression caused by (bug #): 1738173
User impact if declined: crash
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky): Low, code simply ensures that the called function can properly return its result, instead of storing it into a bad location.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/22a302c0c965
Ensure all callers of Pop3/GetNewMail can correctly receive the result from the JS XPCOM implementation. r=rnons.

Comment 18

[Wayne Mery (:wsmwk)]

Comment on attachment 9284182 [details]
Bug 1766169 - Ensure all callers of Pop3/GetNewMail can correctly receive the result from the JS XPCOM implementation. r=rnons.

[Triage Comment]
Approved for beta

Comment 19

[Rob Lemley [:rjl]]

Thunderbird 103.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/6d7a6dc1b9eb

Comment 20

[Rob Lemley [:rjl]]

Comment on attachment 9284182 [details]
Bug 1766169 - Ensure all callers of Pop3/GetNewMail can correctly receive the result from the JS XPCOM implementation. r=rnons.

[Approval Request Comment]
Regression caused by (bug #): 1738173
User impact if declined: crash
Testing completed (on c-c, etc.): 103.0b4
Risk to taking this patch (and alternatives if risky): Low, code simply ensures that the called function can properly return its result, instead of storing it into a bad location.

Comment 21

[Wayne Mery (:wsmwk)]

Comment on attachment 9284182 [details]
Bug 1766169 - Ensure all callers of Pop3/GetNewMail can correctly receive the result from the JS XPCOM implementation. r=rnons.

[Triage Comment]
Approved for esr102

Comment 22

[Rob Lemley [:rjl]]

Thunderbird 102.0.2:
https://hg.mozilla.org/releases/comm-esr102/rev/60e68435d60a

Comment 23

[Wayne Mery (:wsmwk)]

gone everywhere.
Thanks!