Closed Bug 1581079 Opened 6 years ago Closed 1 year ago

Crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader] via nsMsgLocalMailFolder::CopyData - renaming a folder while copy in progress

Categories

(MailNews Core :: Backend, defect, P1)

Product:
MailNews Core
Component:
Backend
Platform:
x86
All
Type:
defect

Tracking

(thunderbird_esr60 wontfix, thunderbird_esr68 affected, thunderbird69 wontfix, thunderbird70 wontfix, thunderbird71 wontfix, thunderbird_esr115 affected, thunderbird_esr128+ wontfix)

Status:
RESOLVED FIXED
Milestone:
132 Branch
Tracking Flags:
Tracking Status
thunderbird_esr60 --- wontfix
thunderbird_esr68 --- affected
thunderbird69 --- wontfix
thunderbird70 --- wontfix
thunderbird71 --- wontfix
thunderbird_esr115 --- affected
thunderbird_esr128 + wontfix

People

(Reporter: wsmwk, Assigned: mkmelin)

References

(Blocks 1 open bug)

Details

(Keywords: crash, topcrash-thunderbird)

Crash Data

Attachments

(1 file, 1 obsolete file)

Bug 1581079 - Try to fix crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader]. r=BenC,kaie
48 bytes, text/x-phabricator-request
Details | Review

A steady crash for version 60, in the top 75, and continuing in version 68

This bug is for crash report bp-9953345c-7a00-49de-9acf-aaa520190913.

Top 10 frames of crashing thread:

0 nss3.dll PL_strncpy nsprpub/lib/libc/src/strcpy.c:25
1 nss3.dll PL_strncat nsprpub/lib/libc/src/strcat.c:29
2 xul.dll nsParseMailMessageState::GetAggregateHeader comm/mailnews/local/src/nsParseMailbox.cpp:899
3 xul.dll nsParseMailMessageState::FinalizeHeaders comm/mailnews/local/src/nsParseMailbox.cpp:1288
4 xul.dll nsParseMailMessageState::ParseFolderLine comm/mailnews/local/src/nsParseMailbox.cpp:680
5 xul.dll nsParseMailMessageState::ParseAFolderLine comm/mailnews/local/src/nsParseMailbox.cpp:663
6 xul.dll nsMsgLocalMailFolder::EndMessage comm/mailnews/local/src/nsLocalMailFolder.cpp:2716
7 xul.dll nsCopyMessageStreamListener::EndMessage comm/mailnews/base/src/nsCopyMessageStreamListener.cpp:77
8 xul.dll nsImapMailFolder::EndMessage comm/mailnews/imap/src/nsImapMailFolder.cpp:5685
9 xul.dll `anonymous namespace'::SyncRunnable2<nsIImapMailFolderSink, nsIImapProtocol*, int>::Run comm/mailnews/imap/src/nsSyncRunnableHelpers.cpp:147

A steady crash for version 60, in the top 75, and continuing in version 68

But virtually zero visibility in betas and nightly, with an average of only one crash per month in betas - so I'm going to assume this crash still exists in 70 and 71.

status-thunderbird69: --- → wontfix
status-thunderbird70: --- → affected
status-thunderbird71: --- → affected
status-thunderbird_esr60: --- → affected

Similar ranking for version 78.2.1

Blocks: 1051616
Severity: critical → S3
Depends on: 1175168

Same ranking for Version 91.2.1 and version 78.14.0 ~#102

So it doesn't seem to have been helped by bug 1175168

Depends on: 1742991

Crash report: https://crash-stats.mozilla.org/report/index/18a6df54-861d-44e2-bfcd-f30870211109 91.3.0

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0 nss3.dll PL_strncpy nsprpub/lib/libc/src/strcpy.c:31
1 nss3.dll PL_strncat nsprpub/lib/libc/src/strcat.c:31
2 xul.dll nsParseMailMessageState::GetAggregateHeader comm/mailnews/local/src/nsParseMailbox.cpp:815
3 xul.dll nsParseMailMessageState::FinalizeHeaders comm/mailnews/local/src/nsParseMailbox.cpp:1170
4 xul.dll nsParseMailMessageState::ParseFolderLine comm/mailnews/local/src/nsParseMailbox.cpp:617
5 xul.dll nsParseMailMessageState::ParseAFolderLine comm/mailnews/local/src/nsParseMailbox.cpp:602
6 xul.dll nsMsgLocalMailFolder::EndMessage comm/mailnews/local/src/nsLocalMailFolder.cpp:2402
7 xul.dll nsCopyMessageStreamListener::EndMessage comm/mailnews/base/src/nsCopyMessageStreamListener.cpp:69
8 xul.dll nsImapMailFolder::EndMessage comm/mailnews/imap/src/nsImapMailFolder.cpp:5247
9 xul.dll `anonymous namespace'::SyncRunnable2<nsIImapMailFolderSink, nsIImapProtocol*, int>::Run comm/mailnews/imap/src/nsSyncRunnableHelpers.cpp:121

Anje, have you had any crashes in the past half year? If yes, what are their crash IDs?

Flags: needinfo?(anjeyelf)

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

Anje, have you had any crashes in the past half year? If yes, what are their crash IDs?

No. So far all is good - Not had any crashes. If I do then I'll post here.

Flags: needinfo?(anjeyelf)

Some reporter comments:

strnlen bp-da7eb498-f0d2-4b4b-9532-4fc8c0231005 reporter writes "I moved a folder, and then quickly moved that folder someplace else"

Crash Signature: [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader] → [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader] [@ strnlen]
Flags: needinfo?(mkmelin+mozilla)
Summary: Crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader] → Crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader] via nsMsgLocalMailFolder::CopyData

Still no clues. I assume one of the pointers got corrupted, but why...

Flags: needinfo?(mkmelin+mozilla)

I don't understand why, but 115.6.0 has gotten worse. Both moving folder to local
bp-0de84184-f283-4eb1-88b2-089a70230822
bp-230eb8a7-0324-49c6-8e1c-0b4d40230821

Is bp-12576576-b43a-4857-8ae3-2a23c0231031 any more useful?

Flags: needinfo?(mkmelin+mozilla)

bp-e618a1b1-cca4-4807-9509-6c0090240213 - renaming a LOCAL mail folder

I PMed bp-e643e546-7744-485e-9d75-ca5a00240122 (steve)

Flags: needinfo?(vseerror)

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

bp-e618a1b1-cca4-4807-9509-6c0090240213 - renaming a LOCAL mail folder

I PMed bp-e643e546-7744-485e-9d75-ca5a00240122 (steve) crash sig for this bug

Steve had many crashes and just as many crash signatures. And also frequently getting "not responding". But was only crashing with 122 beta and newer. (going back to 115 has no crashes) Not very illuminating, except he is running Malwarebytes:

Only two of steve's crashes have bug reports on file, the first (this bug) and the last which is Bug 1872232 - Crash in [@ nsOfflineStoreCompactState::OnStopRequest] - was uplifted at 122.0b3, which is after steve's first crash Thunderbird 122.0b2 Crash Report @ nsOfflineStoreCompactState::OnStopRequest

Flags: needinfo?(vseerror)

steve unfortunately had to leave beta. But perhaps those involved Malwarebytes.

Still #2 crash for 126.0b3. There are no other user contacts.

Keywords: topcrash-thunderbird

Unsuccessful causing a crash by renaming a local folder while copy from imap was in progress, with daily build.

status-thunderbird_esr115: --- → affected
Summary: Crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader] via nsMsgLocalMailFolder::CopyData → Crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader] via nsMsgLocalMailFolder::CopyData - renaming a folder while copy in progress

As can be seen in the graph, the crash rate has increased significantly since July 30. Indeed this is the number #1 crash for esr128, compared to being ranked around #38 for version 115.

I trace the crash rate increase to the neighborhood of version 122, which might warrant a new bug report.

status-thunderbird_esr128: --- → affected
tracking-thunderbird_esr128: --- → +
See Also: → 1890230

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

As can be seen in the graph, the crash rate has increased significantly since July 30. Indeed this is the number #1 crash for esr128, compared to being ranked around #38 for version 115.

I trace the crash rate increase to the neighborhood of version 122, which might warrant a new bug report.

In the last 3 months of crash stats, there no standout user comments, but some do mention messages redownloading and some not displaying all messages, for example bp-c19fc530-fd48-44dd-a7a5-05c900240805 old emails are not showing and just shows a blank screen

One user also crashes bug 1866944

Flags: needinfo?(mkmelin+mozilla)
See Also: → 1866944
See Also: → 877831

This clarifies the type of a few variables. I don't think that is the issue though.

Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED

Some further investigation suggests a shift in crash signatures -

  • in 115.14.0esr the #1 crash is OOM | small by factor of four to the next highest ranking crash signature
  • in 128.1.1esr the #1 crash is now this bug

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

This has been #1 crash for version 128.
The average user crashes approx 1.5 times.

The percentage of Japanese local crashes is quite high - 26% of crashes - https://crash-stats.mozilla.org/search/?signature=%3DPL_strncpy%20%7C%20PL_strncat%20%7C%20nsParseMailMessageState%3A%3AGetAggregateHeader&product=Thunderbird&date=%3E%3D2024-06-05T01%3A21%3A00.000Z&date=%3C2024-09-05T01%3A21%3A00.000Z&_facets=signature&_facets=useragent_locale&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-useragent_locale

If I recall correctly, the # of Japanese users is quite high to begin with.
But I will check the patch if there is any suspicion of multi-byte character, etc. involved in the problem, but in the age of UTF8, that may be relatively low suspect. # number of crash reports from zh-tw and zh-cn users is also low.

BTW, I noticed the failure case of PR_CALLOC() is not handled well and commented on it.

EDIT: layout issue caused by # at the beginning of the post.

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

Some further investigation suggests a shift in crash signatures -

  • in 115.14.0esr the #1 crash is OOM | small by factor of four to the next highest ranking crash signature
  • in 128.1.1esr the #1 crash is now this bug

OOM can be related to the PR_CALLOC() failure (cannot allocate memory), which is not handled well in the current code as I pointed out in the phabricator patch page.

Aha, if PR_CALLOC() fails, we end up with a null pointer in |value| and non-zero |length|, I am sure sometime later this null pointer in value is accessed since |length| is non-zero. Then kaboom. This is one of the reason why it crashes.

mkmelin's comment where setting nullptr was forgotten may be another cause?

Attachment #9422575 - Attachment description: Bug 1581079 - Try to fix crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader]. r=BenC → Bug 1581079 - Try to fix crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader]. r=BenC,Chiaki

Adding tb128found, due to whatever regression has occurred in 128 to make this a topcrash

Blocks: tb128found

The [@ MsgUnhex] crash (covered in Bug 1907773) has the same underlying cause as the [@ strnlen ] signature in this bug, which is very likely corrupted message data, so maybe we should dupe these two together.

See Also: → 1907773

It may be related, though I'm not sure it's a dupe. We can check later if that disappears when we fix this.

Flags: needinfo?(mkmelin+mozilla)

(In reply to Hartmut Welpmann [:welpy-cw] from comment #28)

The [@ MsgUnhex] crash (covered in Bug 1907773) has the same underlying cause as the [@ strnlen ] signature in this bug, which is very likely corrupted message data, so maybe we should dupe these two together.

Why would we be seeing more corrupted message data?

Blocks: 1907773
See Also: 1907773 →
Attachment #9422575 - Attachment description: Bug 1581079 - Try to fix crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader]. r=BenC,Chiaki → Bug 1581079 - Try to fix crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader]. r=BenC
Keywords: checkin-needed-tb
Target Milestone: --- → 132 Branch

Pushed by kaie@kuix.de:
https://hg.mozilla.org/comm-central/rev/35f473611f68
Try to fix crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader]. r=BenC

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Keywords: checkin-needed-tb
Resolution: --- → FIXED
See Also: → 1920051
Pushed by kaie@kuix.de: https://hg.mozilla.org/comm-central/rev/7c0abee63587 Backing out changeset 35f473611f68. rs=bustage
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #9426184 - Attachment is obsolete: true
Attachment #9422575 - Attachment description: Bug 1581079 - Try to fix crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader]. r=BenC → Bug 1581079 - Try to fix crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader]. r=BenC,kaie

Pushed by toby@thunderbird.net:
https://hg.mozilla.org/comm-central/rev/b51cb7634041
Try to fix crash in [@ PL_strncpy | PL_strncat | nsParseMailMessageState::GetAggregateHeader]. r=BenC,kaie

Status: REOPENED → RESOLVED
Closed: 1 year ago → 1 year ago
Keywords: checkin-needed-tb
Resolution: --- → FIXED
Flags: needinfo?(vseerror)

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

Seems the crash is one line off for some reason. Must be 642 instead of 641. https://searchfox.org/comm-central/rev/ced14298b38d818a49b90571fb4d8c04a8668c02/mailnews/local/src/nsParseMailbox.cpp#642

I am just curious. Come to think of it, why are we using PL_strncat() instead of strncat()?
I think all reasonable targets (Windows, linux, MacOSX, etc.) today have sane implementation of
strncat().

I checked the code in PL_strncat() in https://searchfox.org/comm-central/source/mozilla/nsprpub/lib/libc/src/strcat.c#20
PL_strcat in the same file calls strcat if both the string parameters are non-null. So we might use strcat in the first place here.
PL_strncat eventually calls PL_strncpy() defined in https://searchfox.org/comm-central/source/mozilla/nsprpub/lib/libc/src/strcpy.c#20

--- I misread it. The following is a lie.
If I read correctly PL_strncpy does not seem to terminate the resulting data with nul at all (?!).
Whoa!
See the commented out section. https://searchfox.org/comm-central/source/mozilla/nsprpub/lib/libc/src/strcpy.c#34

BUT it still does not fill the remaining area with nul's as strncat does when the data (is nul-termianted) and its string length is less than the third argument. This is what strncat is supposed to do.
Is this intended behavior? It is rather different from strncat drastically.

Now we can argue that |value| is allocated by |PL_CALLOC()| and should start with NUL-filled state,
and so "PL_strncpy() called from PL_strncat() not fill the result with nul when the data is copied as strncpy may do under certain conditions" does not matter in THIS PARTICULAR routine.
But it is very convoluted and can be a maintenance headache. It takes maybe 10 minutes to figure out the internal working of this routine for someone who reads this the first time and verify all the above. It actually takes more than 10 minutes, more like 30 minutes.

Another issue is that we are copying a non-null-terminated strings to an array that is again
handled as a pair of {length, data} in the end, which does not seem to assume null-termination.
So use of bcopy (and its friend) makes it explicit about the nature of data we are handling, and more appropriate here IMHO.

I tried to re-read the code and to be frank, I felt it would be better to use bcopy (and its friend) by keeping track of where to copy the data in each loop.

I suggest rewriting this routine using bcopy (or equivalent such as memcpy) if we still get the strange crashes in this routine,

One other reason I suggest rewriting is this routine is not a speed monster. It reminds me of bubble sort.
Its execution time is not O(length), but somewhere between O(length) and O(length ^ 2) because strncat needs to scan the
area so far filled.
Since this routine is a concatenation operation of "to" or "cc" (and "bcc", too?) addresses, we can say it may not be a big deal unless we have many addresses as mailing list. If we do (like 256 addresses), I think the execution time is large.
But using bcopy (or equivalent) can cut the execution down to O(length) neatly.

Just a thought.

The behavior of |PL_strncpy()| should be verified if it is intended as such. (It diverges from strncpy() when the length of second argument as nul-terminated string is shorter than the third argument, I think.)
Modifying it may not be wise since it seems to be used in so many places.
I think we should replace it with strncpy() (and PL_strncat() with strncat()) where appropriate during maintenance rewrite nearby.

EDIT: PL_strncat() does end the data with nul where appropriate.
But it DOES NOT REPEAT THE FILLING with nul as strncat does under some cases. (That part is conditionalized in the source code) Maybe this is a tiny speed improvement of PL_strncat over strncat in general. In this routine, it is not a win since we ALWAYS copy up to the third argument.
I have no idea if there is a code that depends on this behavior. of PL_strncpy(). We really should stick to the widely adopted standard for long-term maintenance. So I vote for the use of strncat, strncpy, etc. Or even for strlcpy, etc. where appropriate.

Doesn't look like this fixed it unfortunately.
Maybe bug 1920051 will come to the rescue.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Interesting.
I am testing local patches, and my local C-C TB compiled with GCC-14 hits the assertion in
https://searchfox.org/comm-central/source/mailnews/local/src/nsParseMailbox.cpp#904

      MOZ_ASSERT(header->value[header->length] == 0,
                 "Non-null-terminated strings cause very, very bad problems");

This is testing done by DEBUG version of C-C TB and during xpcshell-test.

I have to admit my local C-C tree is not quite up-to-date (it DOES have the patch in comment 34 9 days ago though. So reasonably up-to-date.)
And it also has my other local patches, but I don't believe I tinker with this part of the code and data.
So it may give us some clues what goes on.
I am investigating by dumping the header value data.

There can be genuine bugs or
there are constructs that can be optimized in a funny manner by GCC to hit the assertion.
I wonder what goes on with the clang on treeherder.

Well, as I check there are simply too many tests that hit the assertion.
It seems that header->length is 0 when this happens and header->value[0] contains non-nul character data.
I THINK we may have to set value[0] to NUL when we set length to zero to placate the assertion.

A more investigation follows.

PS: I have not run xpcshell-test often in the last 30 days using a version of C-C TB compiled by GCC.
I held on updating the trees and rather,
I tried to resolve the issue of my mozilla account not quite correctly setup to use the UI panel on treeherder.
So far, to no avail. :-(

(In reply to ISHIKAWA, Chiaki from comment #39)

Interesting.
I am testing local patches, and my local C-C TB compiled with GCC-14 hits the assertion in
https://searchfox.org/comm-central/source/mailnews/local/src/nsParseMailbox.cpp#904

      MOZ_ASSERT(header->value[header->length] == 0,
                 "Non-null-terminated strings cause very, very bad problems");

This is testing done by DEBUG version of C-C TB and during xpcshell-test.

I have to admit my local C-C tree is not quite up-to-date (it DOES have the patch in comment 34 9 days ago though. So reasonably up-to-date.)
And it also has my other local patches, but I don't believe I tinker with this part of the code and data.
So it may give us some clues what goes on.
I am investigating by dumping the header value data.

I think it was a red herring. I updated both M-C and C-C and the symptom is gone.
A good news is due to some mysterious reasons (kernel update, various library upgrade, GCC version upgrade, etc.), the valgrind + C-C TB is working for xpcshell-test.
mochitest is another story, though. Displaying graphics under X11 causes a serious problem now, and there is an issue of context menus not showing is a serious impediment. Bug 1880148

Depends on: 1920051
Flags: needinfo?(vseerror)
See Also: 1920051 →

A percentage of these may be fixed by compact-related Bug 1923526 - Creates multiple Inboxes upon restart (Inbox-1, Inbox-2, Inbox-3, Inbox-4 ...)?

Some related crash comments:

  • bp-fb71cfca-f47a-4f1d-8686-a17bf0241030 2024-10-30 I cannot compact files -- the system created a new "inbox-1" to start to compact, and then gave a message that there was not enough disk space. I have deleted many emails, and moved others to a different folder, but still cannot compact without Thunderbird crashing. I cannot seem to delete that "inbox-1" folder either without a crash.
  • bp-54247744-943f-431b-9932-5b2f20241024 2024-10-24 O thunderbird está a criar pastas Inbox 1, Inbox 2, e seguintes, sempre que eu apago emails ou pastas com emails. E refere não conseguir compactar pasta da caixa de entrada por falta de espaço em disco.
  • bp-56f3798b-3413-4bea-bb45-2159c0241024 2024-10-24 I was compacting folders when Thunderbird crashed. I have been having issues with the inbox displays going blank when I delete a message, and a temporary mailbox file is then created withs ome but not all of the emails. When the program re-opens, all appears to be normal.
  • bp-490219f8-b2d5-4f34-81cb-a6cd10241017 2024-10-17 crash during opening. I export mbox to local folder. Then the emails keep coming out and rename as Gmail-1, Gmail-2 and Gmail.msf.compact-backup.
  • bp-afa8b549-94ab-458c-927e-e6c0e0241010 2024-10-10 compacting large files cause srace conditions - fix please :-)
  • bp-69ca5334-3364-4ede-96e1-7eaa80241007 2024-10-07 Inbox-1 Inbox-2 Inbox-3 Inbox.msf-1.compact-backup Inbox.msf.compact-backup
  • bp-53856c66-d4b7-48bb-b030-7f12b0240923 2024-09-23 Crashed when trying to remove a duplicate folder of duplicate emails when Thunderbird tried to do a "compacting" which keeps crashing my software.
  • bp-67ef466f-30b2-4933-baf3-8a7420240821 2024-08-21 Trying to compact folders but not enough space. This sucks!!!
  • bp-24520228-fc5f-4bf1-b211-f943f0240811 2024-08-11 There's been a problem about compacting folders. A second and third Inbox was created. I wanted to delete Inbox 2 and Thunderbird crashed while trying to do this.

From bug 1920051 comment 2....
Small sample of interesting crash comments for reporters of bug 1581079 - a wide variety of comments, but there certainly are patterns of incorrect behavior beyond the crashes:

Depends on: 1923526

This remains the top crash for 128.4.3esr

But, there is a noticible, significant drop in both signatures. About 30%:

So unless a new crash signature has emerged, there has been an overall decrease caused by some patch(es). But I haven't pinpointed which release(s) helped most, 128.4.0, 128.4.1, 128.4.2, because a) the beta crash rate doesn't seem to have changed in the past 3 months, b) of our crazy updates which result in non-linear adoption of new releases.

1 month of 128.4.0esr, 128.4.1esr, 128.4.2esr, 128.4.3esr

No decrease in crash rates on Nov 8, but there is on Nov 11. Correlating that with version uptake, I think we must conclude 128.4.2 had an impact. So one of these patches.

If the crashes aren't completely gone, but just dropped frequency, maybe related to raising the threshold for triggering folder compact, which might cause that to happen less frequently. Just a wild guess.

Might be gone. We don't have 135+ crashes it seems (bug 1920051 might have fixed).

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

Might be gone. We don't have 135+ crashes it seems (bug 1920051 might have fixed).

Apparently this signature has now moved to memcpy | nsCharTraits<T>::copy…

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

No decrease in crash rates on Nov 8, but there is on Nov 11. Correlating that with version uptake, I think we must conclude 128.4.2 had an impact. So one of these patches.

(In reply to Kai Engert [:KaiE:] from comment #45)

If the crashes aren't completely gone, but just dropped frequency, maybe related to raising the threshold for triggering folder compact, which might cause that to happen less frequently. Just a wild guess.

Ultimately esr crash rate for nsParseMailMessageState::GetAggregateHeader stabilized, present day, at 75% reduction compared to early November. But still not gone in esr where it ranks #2/#3.

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

Might be gone. We don't have 135+ crashes it seems (bug 1920051 might have fixed).

this graph of beta, release, nightly shows a significant drop in early December circa 134.0b1/134.0b2 - way earlier than bug 1920051, but I didn't dig deeper into the data for an explanation. Note, there is a messy mix of 115.nn esr crashes in this query.

That said, both signatures strnlen and nsParseMailMessageState::GetAggregateHeader do indeed die in beta 135.

(In reply to Hartmut Welpmann [:welpy-cw] from comment #47)

Apparently this signature has now moved to memcpy | nsCharTraits<T>::copy…

It does seem so. And perhaps also memcpy_avx_ermsb_Intel | nsCharTraits<T>::copy

But at the same crash rate? If you compare the crash rates of pre-135 against 135+136 (for the two new signatures) it looks to me like the crash rates are about the same - release channel, beta channel

So bug 1920051 only changed the signature?

See Also: → 1954381

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

nsParseMailMessageState::GetAggregateHeader again drops in early February, another 30% for 128

I can't account for this drop.

And for some reason it now decisively ranks #1 in 128.9.1, up from #3 in 128.8.1.

This might be because crash rates of the other top two crashes dropped in 128.9.x. So far.

Flags: needinfo?(vseerror)

Is the existing patch worth uplifting to esr?

Flags: needinfo?(vseerror) → needinfo?(mkmelin+mozilla)

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

This has been #1 crash for version 128.
The average user crashes approx 1.5 times.

The percentage of Japanese local crashes is quite high - 26% of crashes

Even finer points 1) this is the #1 crash for Japanese locale, 2) Japanese for this signature as has double the number of crashes of the next highest locale, which is en-US. Some ja-JP crashes:

Probably not worth uplifting at this stage.

Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(vseerror)

Per comment 47, signature moved. So let's close this and continue in bug 1954381.

status-thunderbird_esr128: affected → wontfix
Status: REOPENED → RESOLVED
Closed: 1 year ago → 1 year ago
Resolution: --- → FIXED
Flags: needinfo?(vseerror)

(In reply to Hartmut Welpmann [:welpy-cw] from comment #28)

The [@ MsgUnhex] crash (covered in Bug 1907773) has the same underlying cause as the [@ strnlen ] signature in this bug, which is very likely corrupted message data, so maybe we should dupe these two together.

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

It (Bug 1907773) may be related, though I'm not sure it's a dupe. We can check later if that disappears when we fix this.

With the roll out of esr140, the decrease in crash rate of this bug from July to August mirrors Bug 1907773. So there is a correlation.

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

Per comment 47, signature moved. So let's close this and continue in bug 1954381.

On the other hand, the crash rate of follow up Bug 1954381 - Crash in [@ memcpy | nsCharTraits<T>::copy] via nsParseMailMessageState::FinalizeHeaders - has greatly increased 2x-3x with version 140

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: