Closed Bug 1264302 Opened 9 years ago Closed 6 years ago

Random crash getting new email in nsImapServerResponseParser::msg_fetch_literal. Made worse by, but not caused by, fetch_by_chunks

Categories

(Thunderbird :: Message Compose Window, defect)

38 Branch
x86_64
Windows NT
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1216951

People

(Reporter: richard.leger, Unassigned)

References

(Blocks 1 open bug, )

Details

(Keywords: crash, steps-wanted, Whiteboard: [need testing with fetch_by_chunks disabled])

Crash Data

User Story

(from bug 1294082)

user environment...
Win7 Pro 64bits
imap autosync DISALBED
roaming profile 
local network, IMAP server Cyrus 
Microsoft Security Essentials antivirus

Attachments

(1 file)

This bug was filed from the Socorro interface and is report bp-697da2f1-3eae-462f-a4d7-e68292160413. =============================================================
Version: unspecified → 38 Branch
Thunderbird 38.7.2 Crash Report [@ nsImapServerResponseParser::msg_fetch_literal ] Search Mozilla Support for Help ID: 697da2f1-3eae-462f-a4d7-e68292160413 Signature: nsImapServerResponseParser::msg_fetch_literal
End user was replying to an email, and when typing in the body of the message, the application crashed unexpectedly. After re-opening application, end-user manage to resume msg from draft and send message.
It happens repeatedly since last week about 5 times per week.
Thanks for the bug report. Not necessary to attach the crash report page - it's nice to access it online. See the link ... bp-697da2f1-3eae-462f-a4d7-e68292160413 0 xul.dll nsImapServerResponseParser::msg_fetch_literal(bool, int) c:/builds/moz2_slave/tb-rel-c-esr38-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:3151 1 xul.dll nsImapServerResponseParser::msg_fetch_content(bool, int, char const*) c:/builds/moz2_slave/tb-rel-c-esr38-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:2157 2 xul.dll nsImapServerResponseParser::mime_part_data() c:/builds/moz2_slave/tb-rel-c-esr38-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:2683 3 xul.dll nsImapServerResponseParser::msg_fetch() c:/builds/moz2_slave/tb-rel-c-esr38-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:1348 4 xul.dll nsImapServerResponseParser::response_data() c:/builds/moz2_slave/tb-rel-c-esr38-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:702 5 xul.dll nsImapServerResponseParser::ParseIMAPServerResponse(char const*, bool, char*) c:/builds/moz2_slave/tb-rel-c-esr38-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:205 6 xul.dll nsImapProtocol::ParseIMAPandCheckForNewMail(char const*, bool) c:/builds/moz2_slave/tb-rel-c-esr38-w32_bld-0000000/build/mailnews/imap/src/nsImapProtocol.cpp:1928 7 xul.dll nsImapProtocol::FetchMessage(nsCString const&, nsIMAPeFetchFields, char const*, unsigned int, unsigned int, char*) c:/builds/moz2_slave/tb-rel-c-esr38-w32_bld-0000000/build/mailnews/imap/src/nsImapProtocol.cpp:3568 Perhaps is same issue as bug 610465. But it might be different line numbers. We should see what happens after user/reporter can test the fix of bug 610465.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Ok I agree. Let's check if Thunderbird 45 (https://www.mozilla.org/en-US/thunderbird/45.0/releasenotes/) that has just been released make a difference as it seems to sort bug 610465 in theory. I will ask end-user to upgrade when possible and report back. Thanks for the tip, I did not realised that the crash report was made available online... great feature! As an unrelated question, if an issue raise within Thunderbird which does not crash application, is there a way to force creation of a crash report to provide information for analyse/bug report? Via some sort of command or button within Thunderbird? I tried to run Thunderbird in log mode but without great success at finding source of issue described in bug 1222691. Could this bug be re-opened? I do have users having this issue and I am ready to work on it with your support.
(In reply to Richard Leger from comment #5) > Ok I agree. > > Let's check if Thunderbird 45 > (https://www.mozilla.org/en-US/thunderbird/45.0/releasenotes/) that has just > been released make a difference as it seems to sort bug 610465 in theory. > > I will ask end-user to upgrade when possible and report back. good results? > As an unrelated question, if an issue raise within Thunderbird which does > not crash application, is there a way to force creation of a crash report to > provide information for analyse/bug report? Via some sort of command or > button within Thunderbird? what issue? if performance, a crash dump is not useful but... > I tried to run Thunderbird in log mode but without great success at finding > source of issue described in bug 1222691. Could this bug be re-opened? I do > have users having this issue and I am ready to work on it with your support. For your performance issue please create a support topic at https://support.mozilla.org/en-US/questions/new and post the URL here
Flags: needinfo?(richard.leger)
(In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #6) > (In reply to Richard Leger from comment #5) > > Ok I agree. > > > > Let's check if Thunderbird 45 > > (https://www.mozilla.org/en-US/thunderbird/45.0/releasenotes/) that has just > > been released make a difference as it seems to sort bug 610465 in theory. > > > > I will ask end-user to upgrade when possible and report back. > > good results? > Sorted by upgrade to TB 45.0 > > > As an unrelated question, if an issue raise within Thunderbird which does > > not crash application, is there a way to force creation of a crash report to > > provide information for analyse/bug report? Via some sort of command or > > button within Thunderbird? > > what issue? > if performance, a crash dump is not useful but... If Thunderbird is not crashing but regularly Not Responding, what is the best way to find out what the issue can be? Safe mode, console, or logs not showing information that could explain issue... Had done some memory dump but Ibdon't have sufficient level of expertise to analyse them... though I could see two dump could be compared... > > I tried to run Thunderbird in log mode but without great success at finding > > source of issue described in bug 1222691. Could this bug be re-opened? I do > > have users having this issue and I am ready to work on it with your support. > > For your performance issue please create a support topic at > https://support.mozilla.org/en-US/questions/new and post the URL here Issue was application not responding randomely, with frequent delay when typing text in composing windows after 3H of use. Seems upgrade to 45.0 version improved situation. Restarting application was sorting issue for a while before it came back. Seems gone in 45.0 version but too early to tell for sure...
(In reply to Richard Leger from comment #7) > (In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #6) > > (In reply to Richard Leger from comment #5) > > > Ok I agree. > > > > > > Let's check if Thunderbird 45 > > > (https://www.mozilla.org/en-US/thunderbird/45.0/releasenotes/) that has just > > > been released make a difference as it seems to sort bug 610465 in theory. > > > > > > I will ask end-user to upgrade when possible and report back. > > > > good results? > > > > Sorted by upgrade to TB 45.0 Then let's close this WFM. (although we do have other crash reports for this signature) For your other issues, I have replied by email
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(richard.leger)
Resolution: --- → WORKSFORME
It seems issue re-occurred... so I re-opened this ticket... Thunderbird 45.2.0 Crash Report [@ nsImapServerResponseParser::msg_fetch_literal ] Search Mozilla Support for Help ID: 126ee39a-735d-4d21-8138-ceecd2160802 Signature: nsImapServerResponseParser::msg_fetch_literal If the bug report not sufficient to identify the issue, is there a way to increase debug log on the client side to find out the reason of issue?
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
bp-126ee39a-735d-4d21-8138-ceecd2160802 0 xul.dll nsImapServerResponseParser::msg_fetch_literal(bool, int) c:/builds/moz2_slave/tb-rel-c-esr45-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:3153 1 xul.dll nsImapServerResponseParser::msg_fetch_content(bool, int, char const*) c:/builds/moz2_slave/tb-rel-c-esr45-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:2158 2 xul.dll nsImapServerResponseParser::mime_part_data() c:/builds/moz2_slave/tb-rel-c-esr45-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:2684 3 xul.dll nsImapServerResponseParser::msg_fetch() c:/builds/moz2_slave/tb-rel-c-esr45-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:1349 4 xul.dll nsImapServerResponseParser::response_data() c:/builds/moz2_slave/tb-rel-c-esr45-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:703 5 xul.dll nsImapServerResponseParser::ParseIMAPServerResponse(char const*, bool, char*) c:/builds/moz2_slave/tb-rel-c-esr45-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:206
> If the bug report not sufficient to identify the issue, is there a way to increase debug log on the client side to find out the reason of issue? As far as I know, not as of today. That makes it frustrating of course. FWIW, beta build crashes for this signature are much lower with version 50. Which doesn't of course mean the issue is fixed. https://crash-stats.mozilla.com/signature/?product=Thunderbird&release_channel=%21release&signature=nsImapServerResponseParser%3A%3Amsg_fetch_literal&date=%3E%3D2016-05-14T17%3A10%3A26.000Z&date=%3C2016-11-14T17%3A10%3A26.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_sort=-version&_sort=-date&page=1 If you want to try the beta, see http://www.mozilla.org/en-US/thunderbird/channel/
Last seen occurrences of the msg_fetch crash issue seems to be sept 2016 TB 45.2... Not seem present in TB 45.3 or 45.4...(yet ;-) Though in the past three months TB 42.2, 42.3 and 42.4 have been crashing on multiple computers (win7b64bits) multiple time per months with various bug reports...
Correction 45.3 and 45.4 versions are affected as well: bp-fc0508c9-2079-4a34-9db2-c4e372161010 10/10/2016 14:51 Thunderbird 45.4.0 Crash Report [@ nsImapServerResponseParser::msg_fetch_literal ] bp-ef1510d0-44e7-4c57-a7fd-1a1992160928 28/09/2016 14:27 Thunderbird 45.3.0 Crash Report [@ nsImapServerResponseParser::msg_fetch_literal ] Will wait for 50 stable version... I am afraid a beta could make things worth...
> Though in the past three months TB 42.2, 42.3 and 42.4 have been crashing on > multiple computers (win7b64bits) multiple time per months with various bug > reports... I meant 45.2, 45.3, 45.4 in sentence above...
FYI, TB still crashing randomely and unexpectedly for various users on various machines when opening an email ... let's hope when TB 50 become stable that will reduce crashes to end-users... Thunderbird 45.5.1 Crash Report [@ nsImapServerResponseParser::msg_fetch_literal ] bp-5e8ce944-f209-484a-aa71-91ce02161207 07/12/2016 13:15 NR https://crash-stats.mozilla.com/report/index/5e8ce944-f209-484a-aa71-91ce02161207 Note: End-user clicked email to open it Thunderbird 45.4.0 Crash Report [@ nsImapServerResponseParser::msg_fetch ] bp-dc4f9fc2-b0f2-4738-982d-a45b52161031 31/10/2016 10:06 MAK bp-e0ff9b00-ca7c-48f3-82c2-ceb2c2161025 25/10/2016 13:59 NR bp-fc0508c9-2079-4a34-9db2-c4e372161010 10/10/2016 14:51 NR Thunderbird 45.3.0 Crash Report [@ nsImapServerResponseParser::msg_fetch_literal ] bp-ef1510d0-44e7-4c57-a7fd-1a1992160928 28/09/2016 14:27 NR bp-4f65ec71-179f-4aee-880f-2276e2160919 19/09/2016 16:49 MM Thunderbird 45.2.0 Crash Report [@ nsImapServerResponseParser::msg_fetch_literal ] bp-cec6145c-39ca-4d1b-bc07-5a5772160920 20/09/2016 12:55 SD bp-bc62b1e9-c958-43c5-a73a-6eb7e2160907 07/09/2016 10:01 MAK bp-d0a23af5-0fc2-4951-afc2-3f89e2160823 23/08/2016 14:33 MM
> let's hope when TB 50 become stable that will reduce crashes to end-users... Version 50 doesn't get a grand public release, nor 51. The next big release is 52. FWIW, the betas are more stable for some users than the current release. I don't like these stacks. What anvirus software do you run?
Keywords: steps-wanted
Summary: Crash in nsImapServerResponseParser::msg_fetch_literal → Random crash in nsImapServerResponseParser::msg_fetch_literal
(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #16) > > let's hope when TB 50 become stable that will reduce crashes to end-users... > > Version 50 doesn't get a grand public release, nor 51. The next big release > is 52. > FWIW, the betas are more stable for some users than the current release. Well will have to wait for the rock solid non-crashing 52 version then :-) one can dream :-) What is the approximate release date planned for version 52, would you know? > I don't like these stacks. Not sure what you mean by this... > What anvirus software do you run? Microsoft Security Essentials
(In reply to Richard Leger from comment #17) > (In reply to Wayne Mery (:wsmwk, NI for questions) from comment #16) > > > let's hope when TB 50 become stable that will reduce crashes to end-users... > > > > Version 50 doesn't get a grand public release, nor 51. The next big release > > is 52. > > FWIW, the betas are more stable for some users than the current release. > > Well will have to wait for the rock solid non-crashing 52 version then :-) > one can dream :-) > What is the approximate release date planned for version 52, would you know? It's a long wait. See the release column at https://wiki.mozilla.org/RapidRelease/Calendar > > I don't like these stacks. > > Not sure what you mean by this... Sometimes the stack for the crash isn't very helpful. Put another way, it is better if a bug report has steps to reliably cause the crash. (but we also know in most cases this is not possible)
URL: 825513
FWIW, from bp-423cc4e5-069a-4c50-b4fb-edbbf2170123 0 xul.dll nsImapServerResponseParser::msg_fetch_literal(bool, int) c:/builds/moz2_slave/tb-rel-c-esr45-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:3153 1 xul.dll nsImapServerResponseParser::msg_fetch_content(bool, int, char const*) c:/builds/moz2_slave/tb-rel-c-esr45-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:2158 2 xul.dll nsImapServerResponseParser::mime_part_data() c:/builds/moz2_slave/tb-rel-c-esr45-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:2684 3 xul.dll nsImapServerResponseParser::msg_fetch() c:/builds/moz2_slave/tb-rel-c-esr45-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:1349 4 xul.dll nsImapServerResponseParser::response_data() c:/builds/moz2_slave/tb-rel-c-esr45-w32_bld-0000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:703 https://hg.mozilla.org/releases/comm-esr45/annotate/tip/mailnews/imap/src/nsImapServerResponseParser.cpp#l2158 if (PL_strcasecmp(fNextToken, "NIL")) { if (*fNextToken == '"') fLastChunk = msg_fetch_quoted(); else 2158 fLastChunk = msg_fetch_literal(chunk, origin); https://hg.mozilla.org/releases/comm-esr45/annotate/tip/mailnews/imap/src/nsImapServerResponseParser.cpp#l3153 *displayEndOfLine = 0; fServerConnection.HandleMessageDownLoadLine(fCurrentLine, specialLineEnding || !lastChunk); *displayEndOfLine = saveit; lastCRLFwasCRCRLF = (*(displayEndOfLine - 1) == '\r'); } else { 3153 lastCRLFwasCRCRLF = (*(fCurrentLine + strlen(fCurrentLine) - 1) == '\r');
User Story: (updated)
Blocks: 1333031
Richard, please try this from bug 434054 comment 9 lockPref("mail.imap.fetch_by_chunks", false); lockPref("mail.server.default.fetch_by_chunks", false); lockPref("mail.server.default.mime_parts_on_demand", false);
This is do be set in Options > Advanced > General > Config Editor right?
Depends on: 1284302
(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #20) > Richard, please try this from bug 434054 comment 9 > > lockPref("mail.imap.fetch_by_chunks", false); > lockPref("mail.server.default.fetch_by_chunks", false); > lockPref("mail.server.default.mime_parts_on_demand", false); Richard, did this help?
Flags: needinfo?(richard.leger)
Haven't gt a chance to implement it yet, keep you posted...
Blocks: 1323596
Blocks: 1333027, 1333037
Blocks: 1333040, 1333032
Whiteboard: [need testing with fetch_by_chunks disabled]
Sorry guys, I just realised that this crash was reported 11 months ago and I cannot find the computer that was encountering the issue to set the settings as advised to test in TB... I would say for now we can consider it closed as since then TB has been updated many times... Unless you would want me to test those settings on a different version of TB, I currently use 52 beta channel... Let me know. Happy to help.
Flags: needinfo?(richard.leger)
Definitely test this on other computers that are crashing, regardless of OS and version, that also crash and are loading the same emails. And we'd leave the bug open anyway, because other users are seeing crashes, including version 52 https://crash-stats.mozilla.com/signature/?release_channel=%21release&signature=nsImapServerResponseParser%3A%3Amsg_fetch_literal&date=%3E%3D2016-12-10T14%3A53%3A10.000Z&date=%3C2017-03-10T14%3A53%3A10.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_sort=-date&page=1
Summary: Random crash in nsImapServerResponseParser::msg_fetch_literal → Random crash in nsImapServerResponseParser::msg_fetch_literal. Related to fetch_by_chunks?
(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #22) > (In reply to Wayne Mery (:wsmwk, NI for questions) from comment #20) > > Richard, please try this from bug 434054 comment 9 > > > > lockPref("mail.imap.fetch_by_chunks", false); Not available in TB 45.8 but implemented ("mail.imap.mime_parts_on_demand", false) instead > > lockPref("mail.server.default.fetch_by_chunks", false); Implemented > > lockPref("mail.server.default.mime_parts_on_demand", false); Implemented > Richard, did this help? Keep you posted :-)
Just to confirm, it seems that Thunderbird has not crashed since the options above were implemented. Current user TB version is 45.8.0.
What can you make of comment 19? Or, is bug 1222455 a better case to look at?
Blocks: 1222455
Flags: needinfo?(m_kato)
Keywords: crash
(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #28) > What can you make of comment 19? I don't understand the twisted logic in that code: 3153 lastCRLFwasCRCRLF = (*(fCurrentLine + strlen(fCurrentLine) - 1) == '\r'); will crash if fCurrentLine is null. I see no evidence that this is guaranteed to be non-null. It would be good to see a reproducible case before adding any further checks here. > Or, is bug 1222455 a better case to look at? I don't think bug 1222455 has got anything to do with this.
(In reply to Jorg K (GMT+2) from comment #29) > (In reply to Wayne Mery (:wsmwk, NI for questions) from comment #28) > > What can you make of comment 19? > I don't understand the twisted logic in that code: > 3153 lastCRLFwasCRCRLF = (*(fCurrentLine + strlen(fCurrentLine) - 1) == > '\r'); > will crash if fCurrentLine is null. I see no evidence that this is > guaranteed to be non-null. It would be good to see a reproducible case > before adding any further checks here. If null, "charsReadSoFar += strlen(fCurrentLine);" may cause crash at first. But we should add nullptr check for safety. (In reply to Wayne Mery (:wsmwk, NI for questions) from comment #28) > Or, is bug 1222455 a better case to look at? Bug 1222455 will be that mMsgWindow is invalid in nsMessenger... This isn't same.
Flags: needinfo?(m_kato)
jpr is another user who crashes bp-83dd2c8b-5494-4e54-af58-8ad410171003 nsMsgLineStreamBuffer::ReadNextLine bp-d89f4ffd-1ba7-44ab-bba2-748be0170928 nsImapServerResponseParser::msg_fetch_content bp-0623b806-6852-4571-a0c9-82dab0170927 nsImapProtocol::GetMessageSize bp-58d8c916-1118-4501-b715-05a450170920 nsImapProtocol::HandleMessageDownLoadLine bp-0538f4ac-5319-48ae-9146-fb4bc0170913 nsImapServerResponseParser::msg_fetch_literal bp-1303ab94-27f1-4ad7-bb30-02aab0170907 _invalid_parameter
Blocks: 1333038
Crash Signature: [@ nsImapServerResponseParser::msg_fetch_literal] → [@ nsImapServerResponseParser::msg_fetch_literal] [@ _invalid_parameter ]
crash rank in 52 has dropped toward #140 > If null, "charsReadSoFar += strlen(fCurrentLine);" may cause crash at first. But we should add nullptr check for safety. Can we add this check as the first step?
Blocks: 1333342
Hmm, I'm really confused, is there a recent crash ID pointing to the crash you want to fix? I need an up-to-date line number. I looked myself https://crash-stats.mozilla.com/signature/?signature=nsImapServerResponseParser%3A%3Amsg_fetch_literal&date=%3E%3D2017-11-26T20%3A24%3A07.000Z&date=%3C2017-12-03T20%3A24%3A07.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-date&page=1#reports and found some recent crashes: bp-0472722a-2872-49bf-b797-f92fc0171203 - TB 52.5, crashed today on nsImapServerResponseParser.cpp:3153 which is lastCRLFwasCRCRLF = (*(fCurrentLine + strlen(fCurrentLine) - 1) == '\r'); bp-e4998124-a5d3-427c-9ef8-f4e540171201 - TB 58 b1, crashed a few days ago on nsImapServerResponseParser.cpp:3135 which is lastCRLFwasCRCRLF = (*(fCurrentLine + strlen(fCurrentLine) - 1) == '\r'); so the same line which is here: https://dxr.mozilla.org/comm-central/rev/5c41c835bbc316c5f50e095741f75ccd4841c2fc/mailnews/imap/src/nsImapServerResponseParser.cpp#3135 So I don't understand how the comment > If null, "charsReadSoFar += strlen(fCurrentLine);" may cause crash at first. > But we should add nullptr check for safety. would apply here. Besides, that line is at https://dxr.mozilla.org/comm-central/rev/5c41c835bbc316c5f50e095741f75ccd4841c2fc/mailnews/imap/src/nsImapServerResponseParser.cpp#3115 and *fCurrentLine is accessed before in 3104 if (lastCRLFwasCRCRLF && (*fCurrentLine == '\r')) so 3115 charsReadSoFar += strlen(fCurrentLine); shouldn't crash. Makoto-san, can you please clarify. I don't see how line 3135 can crash when 3104 and 3115 didn't crash. What am I missing?
No longer blocks: 1333342
Flags: needinfo?(m_kato)
Blocks: 1333342
(In reply to Jorg K (GMT+1) from comment #33) > Besides, that line is at > https://dxr.mozilla.org/comm-central/rev/ > 5c41c835bbc316c5f50e095741f75ccd4841c2fc/mailnews/imap/src/ > nsImapServerResponseParser.cpp#3115 > and *fCurrentLine is accessed before in > 3104 if (lastCRLFwasCRCRLF && (*fCurrentLine == '\r')) > so > 3115 charsReadSoFar += strlen(fCurrentLine); > shouldn't crash. A comment from a passer-by. If lastCRLFwasCRCRLF is false, fCurrentLine on 3104 is not accessed. So it is possible that the fCurrentLine on 3115 can be the first access.
Oh, good catch! Thanks. So check fCurrentLine before line 3115, and if null, do what? Continue, break, error?
Flags: needinfo?(m_kato)
(In reply to Jorg K (GMT+1) from comment #35) > Oh, good catch! Thanks. > > So check fCurrentLine before line 3115, and if null, do what? Continue, > break, error?
Flags: needinfo?(m_kato)
Flags: needinfo?(ishikawa)
(In reply to Wayne Mery (:wsmwk) from comment #36) > (In reply to Jorg K (GMT+1) from comment #35) > > Oh, good catch! Thanks. > > > > So check fCurrentLine before line 3115, and if null, do what? Continue, > > break, error? I am trying to figure this out, Wayne. But I found the code in this file sucks. The code parses the command and response of Imap protocol. However, the code is handcrafted in a somewhat ad-hoc manner. Error handling is not uniform or robust as we found here. I would prefer to have an automated parser created by a parser generator such as bison since that type of parser is very robust and it gives a very high-level control of the parser when a parse error (i.e., incorrect response from the remote server) occurs. I will keep an eye on the rewriting possibility while I try to figure out what course of action should take place. (We may have to combine a lexical analyzer generated by a generator such as lex/flex for really robust parser of the IMAP protocol with good protection in case of parse error, i.e. command/response incompatibilities, etc.) TIA
Flags: needinfo?(ishikawa)
When running this code, fCurrentLine isn't nullptr. charsReadSoFar += strlen(fCurrentLine); But when running this code, fCurrentLine seems to be nullptr. lastCRLFwasCRCRLF = (*(fCurrentLine + strlen(fCurrentLine) - 1) == '\r'); So If PercentProgressUpdateEvent calls any method that updates fCurrentLine, fCurrentLine becomes nullptr. But I don't know root cause.
Flags: needinfo?(m_kato)
Gene is busily rewriting the chunking logic in bug 1216951, I'll make him aware. Gene, the lastCRLFwasCRCRLF goes with your patch in bug 1216951, so let's see whether this crash here goes as well. NI just to attract attention ;-)
Flags: needinfo?(gds)
Thanks, will watch for problems here. (NI still on.)
Crash Signature: [@ nsImapServerResponseParser::msg_fetch_literal] [@ _invalid_parameter ] → [@ nsImapServerResponseParser::msg_fetch_literal] [@ _invalid_parameter ] [@ strlen | nsImapServerResponseParser::msg_fetch_literal ]
Depends on: 1216951
Most recent crashed in TB 52.6 of 2018-02-04 crash here: nsImapServerResponseParser.cpp:3144 In the c-esr52 repository that's char *displayEndOfLine = (fCurrentLine + strlen(fCurrentLine) - (charsReadSoFar - numberOfCharsInThisChunk)); https://hg.mozilla.org/releases/comm-esr52/annotate/acc1e1efc4f4/mailnews/imap/src/nsImapServerResponseParser.cpp#l3144 That code is up for replacement in bug 1216951.
(In reply to Jorg K (GMT+1) from comment #39) > Gene is busily rewriting the chunking logic in bug 1216951, I'll make him > aware. > > Gene, the lastCRLFwasCRCRLF goes with your patch in bug 1216951, so let's > see whether this crash here goes as well. NI just to attract attention ;-) Submitted patch in bug 1216951. In my testing and usage with patch, haven't seen any crashes.
Flags: needinfo?(gds)
Blocks: 1444201
I don't know if it's feasible, but it would be
Blocks: 1294074
(In reply to Wayne Mery (:wsmwk) from comment #44) > I don't know if it's feasible, but it would be Hmm, incomplete thought and I don't remember what I intended. Richard, it would be fantastic if you could test version 60 beta and give us a quick report. It should be out milddle of this week at http://www.mozilla.org/en-US/thunderbird/channel/
Flags: needinfo?(richard.leger)
I will install 60 beta when available... on my laptop I dont have roaming profile enable and I mostly use TB outside local network for email remote location at the moment... will send crash reports that may occur...
Flags: needinfo?(richard.leger)
(In reply to Richard Leger from comment #46) > will send crash reports that may occur... Also, exactly what was been done when it crashes would be helpful. Thanks for testing!
Blocks: 1294082
(In reply to gene smith from comment #47) > (In reply to Richard Leger from comment #46) > > will send crash reports [with beta 60] that may occur... > > Also, exactly what was been done when it crashes would be helpful. Thanks > for testing! Richard, Note - doesn't matter if you are using roamning
Flags: needinfo?(richard.leger)
Wayne, The line where the crash occurs (see Comment 42 above) actually occurs even when not fetching in chunks, although it probably occurs more often when you are. So I have my doubts that the changes in bug 1216951 will have any effect on the crash frequency.
(In reply to gene smith from comment #47) > (In reply to Richard Leger from comment #46) > > will send crash reports that may occur... > > Also, exactly what was been done when it crashes would be helpful. Thanks > for testing! What was done, to generate the crash originally reported, is indicated in Comment 2.
Flags: needinfo?(richard.leger)
(In reply to Wayne Mery (:wsmwk) from comment #50) > (In reply to gene smith from comment #47) > > (In reply to Richard Leger from comment #46) > Richard, > Note - doesn't matter if you are using roamning What this a question to me? or a comment?
(In reply to Richard Leger from comment #53) > (In reply to Wayne Mery (:wsmwk) from comment #50) > > (In reply to gene smith from comment #47) > > > (In reply to Richard Leger from comment #46) > > > Richard, > > Note - doesn't matter if you are using roamning > > What this a question to me? or a comment? That was just a comment, because iirc you use roaming. Even if not, nevermind. like bug 1333038 this signature still exists on beta. But rare.
(In reply to gene smith from comment #51) > Wayne, The line where the crash occurs (see Comment 42 above) actually > occurs even when not fetching in chunks, although it probably occurs more > often when you are. (In reply to Wayne Mery (:wsmwk) from comment #48) > looking at past several months of crash-stats I can't clearly tell if crash > rate dropped as a result of bug 1216951 "Fix broken handling of split CR and > LF between chunks" being patched in version 60. It might have dropped, might > not. But clearly the crash signature still appears, so we have more work to > do. bp-b096a822-5549-47b5-8351-232440180430 60.0b4 This beta+nightly graph from a longer period is a bit more conclusive https://crash-stats.mozilla.com/signature/?release_channel=%21release&signature=nsImapServerResponseParser%3A%3Amsg_fetch_literal&date=%3E%3D2017-12-17T13%3A03%3A45.000Z&date=%3C2018-06-17T15%3A03%3A45.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-date&page=1#graphs So the crash rate is perhaps half and perhaps that is where bug bug 1216951 helped. Richard (our bellweather case) can you confirm whether you and your users still crash, or crash less, when using beta?
Flags: needinfo?(richard.leger)
Summary: Random crash in nsImapServerResponseParser::msg_fetch_literal. Related to fetch_by_chunks? → Random crash getting new email in nsImapServerResponseParser::msg_fetch_literal. Made worse by, but not caused by, fetch_by_chunks
(In reply to Wayne Mery (:wsmwk) from comment #55) > > Richard (our bellweather case) can you confirm whether you and your users > still crash, or crash less, when using beta? That is, with the original settings which caused the crash mentioned in comment 26
Gene, another beta user is Ravi, who has four crash signatures: * nsImapServerResponseParser::msg_fetch_literal bp-7a74ef85-6565-40a7-a234-2938b0180612 C:/builds/moz2_slave/tb-rel-c-beta-w32_bld-00000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:3120 * nsImapProtocol::HandleMessageDownLoadLine bp-a2b84766-5de2-485b-a442-385430180612 comm/mailnews/imap/src/nsImapProtocol.cpp:3996 - Bug 1444389 * nsImapServerResponseParser::ProcessOkCommand bp-73ef8797-7158-4b0e-8f76-ece930180606 C:/builds/moz2_slave/tb-rel-c-beta-w32_bld-00000000/build/mailnews/imap/src/nsImapServerResponseParser.cpp:435 - Bug 628646 * nsImapProtocol::GetMessageSize bp-40a1298e-62a4-45a8-81ca-94d9d0180605 C:/builds/moz2_slave/tb-rel-c-beta-w32_bld-00000000/build/mailnews/imap/src/nsImapProtocol.cpp:4580 - Bug 825513 *** THIS IS THE BUG WITH THE HIGHEST CRASH RATE *** Richard in Bug 825513 Comment 25 reports these crashes are gone (but that was two months ago). But the signature is still a solid topcrash for 60 betas and version 52. Gene, which signature / bug report do you think is best to research? (I have noted Bug 825513 "needs deep code inspection")
Flags: needinfo?(gds)
(In reply to Wayne Mery (:wsmwk) from comment #56) > (In reply to Wayne Mery (:wsmwk) from comment #55) > > > > Richard (our bellweather case) can you confirm whether you and your users > > still crash, or crash less, when using beta? > > That is, with the original settings which caused the crash mentioned in > comment 26 I am still using beta (since about 3 months) and I haven't had crash related to this bug. Only one or two related to an LDAP issue reported in a separate bug report. I have now reset the setting to their original default value in my TB. Keep you posted if any crash occurs in the future. I haven't hears from users in a long time but I would not be able to recall if it stopped following settings changes, TB ugrade or else... it has been too long :-) I will check on my setup and report back.
Flags: needinfo?(richard.leger)
It seems like this was fixed by bug 1216951
Status: REOPENED → RESOLVED
Closed: 9 years ago6 years ago
Flags: needinfo?(gds)
Resolution: --- → DUPLICATE
Depends on: 1494764
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: