1917447 - Crash in [@ MboxMsgInputStream::PumpData] on newsgroup article

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Wayne Mery (:wsmwk)]

All crashes are version 128

Crash report: https://crash-stats.mozilla.org/report/index/747dafa1-e1c0-47c6-b35f-f8e3c0240906 user reports "Opening a news message which contains 5 attachments. What exactly is causing the problem (the message, the attachments, ...) I don't know."

MOZ_CRASH Reason: MOZ_RELEASE_ASSERT(aStart <= len && (aLength == dynamic_extent || (aStart + aLength <= len)))

Top 10 frames:

0  xul.dll  mozilla::Span<const char, 18446744073709551615>::Subspan(unsigned long long, ...  mfbt/Span.h:679
0  xul.dll  MboxMsgInputStream::PumpData()  mailnews/base/src/MboxMsgInputStream.cpp:0
1  xul.dll  MboxMsgInputStream::Available(unsigned long long*)  mailnews/base/src/MboxMsgInputStream.cpp:665
2  xul.dll  XPTC__InvokebyIndex()  /builds/worker/workspace/obj-build/toolkit/library/build/Z:/builds/worker/checkouts/gecko/xpcom/reflect/xptcall/md/win32/xptcinvoke_asm_x86_64.asm:97
3  xul.dll  CallMethodHelper::Invoke()  js/xpconnect/src/XPCWrappedNative.cpp:1620
3  xul.dll  CallMethodHelper::Call()  js/xpconnect/src/XPCWrappedNative.cpp:1174
3  xul.dll  XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode)  js/xpconnect/src/XPCWrappedNative.cpp:1120
4  xul.dll  XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)  js/xpconnect/src/XPCWrappedNativeJSOps.cpp:966
5  xul.dll  CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::...  js/src/vm/Interpreter.cpp:481
5  xul.dll  js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstru...  js/src/vm/Interpreter.cpp:575

mozilla::DirectedAcyclicGraph<T>::GetDirectPredecessors has a same crash reason.

Comment 1

[Alfred Peters [:infofrommozilla]]

Yes, I can reproduce the crash:
132.0a1 (2024-09-07) (64-bit): bp-cc317332-6854-4e89-ac67-764910240908

I have to admit that I also saw the crash back in April:
125.0b1: bp-ee6f09c2-1afe-4fba-a8f2-c04380240402
126.0a1: bp-7f948d39-78ae-4f94-a7e7-219760240402

However, since I could only reproduce it with extremely large articles, which are unusual in normal Usenet, I didn't continue to investigate it.

STR:
You need a group where the option:

 Context menu -> Properties -> /Synchronization\
   => [x] Select this folder for offline use

is activated.
If I display an article with an attachment larger than 718 kB (-> article size 986 kB) in this group, TB crashes.
Sometimes the article is displayed correctly on the 1st viewing and only crashes on the 2nd viewing. Maybe because it has not yet been saved offline???
An article with an attachment of 677 kB (->article size: 930 kB) doesn't yet crash.

Comment 2

[Alfred Peters [:infofrommozilla]]

Possibly a brother of Bug 1843752?
See also: Bug 1881964

(1(1¢)

Comment 3

[Alfred Peters [:infofrommozilla]]

By compiling a debug build without optimization, I was able to determine a few values. Depending on the size of the article, the crash starts from two different places. The actual crash is of course always the assertion in Span.h:

https://searchfox.org/comm-central/rev/b8013810ef3980260a9826dc69b3289a38c32d06/mozilla/mfbt/Span.h#679

  MOZ_RELEASE_ASSERT(aStart <= len && (aLength == dynamic_extent ||
                                         (aStart + aLength <= len)));

case	aStart	len	aLength	dynamic_extent	fault
A	15392	8192	496	-1	aStart > len
B	0	8192	15888	-1	aStart + aLength > len

Case A comes from here:
https://searchfox.org/comm-central/rev/b8013810ef3980260a9826dc69b3289a38c32d06/mailnews/base/src/MboxMsgInputStream.cpp#829

       auto unused = mBuf.AsSpan().Subspan(mUsed, mUnused);

with mBuf.mLength=8192 / mUsed=15392 / mUnused=496

While case B comes from:
https://searchfox.org/comm-central/rev/b8013810ef3980260a9826dc69b3289a38c32d06/mailnews/base/src/MboxMsgInputStream.cpp#848

    auto data = mBuf.AsSpan().Subspan(mUsed, mUnused);

strangely enough, with identical values as above.
mBuf.mLength=8192 / mUsed=15392 / mUnused=496

Since mBuf has a static size of 8192 bytes,

https://searchfox.org/comm-central/rev/b8013810ef3980260a9826dc69b3289a38c32d06/mailnews/base/src/MboxMsgInputStream.cpp#697

      mBuf(8192),

... >15000 bytes can hardly be used there.
See the comment on MboxMsgInputStream::PumpData()
https://searchfox.org/comm-central/rev/b8013810ef3980260a9826dc69b3289a38c32d06/mailnews/base/src/MboxMsgInputStream.cpp#803

Comment 4

[Ben Campbell]

Alfred: thanks for the great investigative work!
There's definitely something bad happening.

I'm still trying to reproduce it here (got any recommendations for usenet servers to use?), but here are my thoughts in the meantime:

My guess is that it's the parser screwing up. data = mParser->Feed(data) feeds in a chunk, and returns the unconsumed portion.
I'm thinking that it's somehow returning a bigger span than was passed in!
Until I can replicate it myself I'll work up a patch to pepper some asserts around, so we can at least catch it when it happens.

In the meantime, if you turn on verbose mbox logging, it'll show you how many bytes it's feeding into the parser each time:
MOZ_LOG="mbox:5"
The log statement here: https://searchfox.org/comm-central/rev/b8013810ef3980260a9826dc69b3289a38c32d06/mailnews/base/src/MboxMsgInputStream.cpp#103
If the size ever goes over 8192 (the size of mBuf), then something has gone wrong! Probably the previous call to Feed() screwed it up.
Might throw up some clue as to what the offending data looks like.

It's odd that it's only happening for News. The mbox code shouldn't care what's reading or writing (or even what the contents of the data are!).
I'm guessing there's some pathological case in new posts with big attachments that the normal email storage doesn't trigger...

Comment 5

[Ben Campbell]

I think this is a case of Bug 1931863 biting us. The patch over there seems to fix this bug for me. It hasn't landed yet, but should do soon.

In any case, my steps to reproduce:

1. start up TB with no profile
2. skip the setup screen and start without any email accounts
3. Menu -> "New Account" -> "Newsgroup"
4. Set up your news provider (I signed up for the free 1GB trial on usenext.com)
5. close the account setup tab
6. rightclick on your new usenet account and click "Subscribe"
7. find a big binaries newsgroup and subscribe to it (I used alt.binaries.video.music)
8. close the subscription dialog
9. click on the newsgroup - just download the first 500 headers.
10. rightclick on the newsgroup -> "Properties" -> "Synchronization" tab -> check "Select this newsgroup for offline use"
11. click through the first few messages, giving them time to download and display
12. go back and click on the first few messages again, cycling through them.

For me step 11 was pretty reliable on causing the crash - usually after just a few clicks.

And with the Bug 1931863 patch installed, this crash no longer happens for me.

Comment 6

[Ben Campbell]

The patches in Bug 1931863 have now landed, and work for me.
Alfred - are you able to try it out too?

Comment 7

[Alfred Peters [:infofrommozilla]]

(In reply to Ben Campbell from comment #6)

The patches in Bug 1931863 have now landed, and work for me.
Alfred - are you able to try it out too?

Yes, I was able to post a 10 MByte zip file locally with the current Daily (Build-ID 20250108103142) (article size: 13.4 MByte) and load it again.
The zip was intact afterwards.