1729778 - Crash in [@ InvalidArrayIndex_CRASH | nsMsgLocalMailFolder::BeginCopy] | ElementAt(aIndex = -1, aLength = 2)

Status: RESOLVED FIXED

(Thunderbird :: Filters, defect, P1)

Description

[Bob Clary [:bc] (inactive)]

Maybe Fission related. (DOMFissionEnabled=1)

Crash report: https://crash-stats.mozilla.org/report/index/ae020f9c-fa81-4ff8-a285-73e620210908

MOZ_CRASH Reason: ElementAt(aIndex = 18446744073709551615, aLength = 2)

Top 10 frames of crashing thread:

0 libxul.so InvalidArrayIndex_CRASH xpcom/ds/nsTArray.cpp:28
1 libxul.so nsMsgLocalMailFolder::BeginCopy comm/mailnews/local/src/nsLocalMailFolder.cpp:1846
2 libxul.so nsCopyMessageStreamListener::OnStartRequest comm/mailnews/base/src/nsCopyMessageStreamListener.cpp:57
3 libxul.so mozilla::net::nsStreamListenerTee::OnStartRequest netwerk/base/nsStreamListenerTee.cpp:24
4 libxul.so  comm/mailnews/imap/src/nsSyncRunnableHelpers.cpp:98
5 libxul.so mozilla::RunnableTask::Run xpcom/threads/TaskController.cpp:502
6 libxul.so mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:805
7 libxul.so mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:641
8 libxul.so mozilla::TaskController::ProcessPendingMTTask xpcom/threads/TaskController.cpp:425
9 libxul.so mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal xpcom/threads/nsThreadUtils.h:531

Over the weekend I started crashing on start up when retrieving email. It turns out that it happens when the filters automatically run after receiving email. Looking in socorro this has happened to others on Linux, MacOS and Windows.

Last good revision:

BuildID=20210903050858
SourceRepository=https://hg.mozilla.org/comm-central
SourceStamp=1ed6dbbb95c2be112d66ec3afcc359f62fbc51b7
ID={3550f703-e582-4d05-9a08-453d09bdfdc6}

First bad revision:

BuildID=20210903104659
SourceRepository=https://hg.mozilla.org/comm-central
SourceStamp=b56ce92dbc14fb3a55f771d6a85e89e73e6f25d0
ID={3550f703-e582-4d05-9a08-453d09bdfdc6}

Regression range 20210903050858-20210903104659

I'll disable Fission and see if it is still reproducible. I've tried to reproduce with the original messages after moving them back but it it isn't clear if the lack of a crash is meaningful or if the messages have changed in the round trip. I'll run Daily tip again without Fission and see if it ever reproduces.

Comment 1

[Wayne Mery (:wsmwk)]

Thanks for reporting this!

I confirm the first crash reports are from buildid 20210903104659, 93.0a1 9/3/2021. And it is #1 crash for 93.0a1 and 94.0a1, so potentially a blocker for shipping 93.0b1. Several checkins that morning.

Comment 2

[Ben Campbell]

My bet would be on:

https://hg.mozilla.org/comm-central/rev/32e946603b4367f77ecc72c61d7796667dbc7b17

Rereading the comment I removed in that patch, I think the code was relying on it failing there under some circumstances.

-  nsCOMPtr<nsISeekableStream> seekableStream =
-      do_QueryInterface(mCopyState->m_fileStream, &rv);
-
-  //  XXX ToDo: When copying multiple messages from a non-offline-enabled IMAP
-  //  server, this fails. (The copy succeeds because the file stream is created
-  //  subsequently in StartMessage) We should not be warning on an expected
-  //  error. Perhaps there are unexpected consequences of returning early?
-  NS_ENSURE_SUCCESS(rv, rv);
-  seekableStream->Seek(nsISeekableStream::NS_SEEK_END, 0);

I think it was using seekableness to detect destinations on non-offline-enabled IMAP folders, which is a terrible method.
So with that code block removed it all fails horribly because it now fails to fail. Sigh.

For now, let's back out that commit. I'll open another bug to remove the seekable madness and fix it properly.

Comment 3

[Ben Campbell]

Attachment: WIP: Bug 1729778: Backed out changeset 32e946603b43 (Bug 1719996) for breaking some msg copying. rs=backout

Comment 4

[Ben Campbell]

Gah. moz-phab marked it "WIP". Not quite sure how backouts are meant to be handled... I could land it myself, but it'd be nice if someone could just sanity check (and/or review it if that's appropriate).

Comment 5

[Magnus Melin [:mkmelin]]

Yeah let's back out. Of course the crashing code is rather odd and should deal with getting -1 for an index ... https://searchfox.org/comm-central/rev/f460fb11697497d39d4a7f354c815b9187cff171/mailnews/local/src/nsLocalMailFolder.cpp#1841-1846

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/65c772fe13f7
Backed out changeset 32e946603b43 (Bug 1719996) for breaking some msg copying. rs=backout

Comment 7

[Wayne Mery (:wsmwk)]

Comment on attachment 9240160 [details]
WIP: Bug 1729778: Backed out changeset 32e946603b43 (Bug 1719996) for breaking some msg copying. rs=backout

Bob, thanks for bringing this to our attention.

[Triage Comment]
approved for beta

I suspect message filters may be one trigger, because my windows instance crashes consistently. Definitely a release blocker, and something we must take on 93 beta.

Comment 8

[Rob Lemley [:rjl]]

Thunderbird 93.0b1:
https://hg.mozilla.org/releases/comm-beta/rev/b306b71dc29e

Comment 9

[Ben Campbell]

I've been playing about with this and can now replicate the same crash, if I add the backed-out patch back in.
It happens when multiple incoming messages are automatically moved by a filter.
A single incoming message doesn't trigger it - has to be more than one.
My setup was using an IMAP account (local dovecot setup - so I can just copy dummy emails files directly into the dovecot "cur" dir to deliver them).
I had a filter set up to move all messages into a local folder.
With the patch backed out, it works as expected.

I think my diagnosis in Comment 2 wasn't quite right (we're not dealing with an IMAP outputstream), but it's obviously some change in that patch which causes it. I'll work on it over at Bug 1719996.

Comment 10

[Wayne Mery (:wsmwk)]

verified, no crash with 93.0b1 build 2 with backout