1373493 - TB 52.2.0 is broken with Gmail IMAP (regression)

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Ache]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170608105825

Steps to reproduce:

Just start TB. I have few Gmail accs with OAuth2 auth.


Actual results:

First symptoms are: some error window appears telling that id<something> IMAP command is invalid or something like. After that whole folders structure becomes broken, all folders are missing, excepting Inbox and new folder [Gmail]^Junk is created. All messages are redownloaded. I have great amount of crashes attempting to change something in the settings and lost columns order after each crash.


Expected results:

I want it simple works like previous 52.1.1. This is obvious regression. All becomes normal on reinstalling 52.1.1

Comment 1

[u597066]

Ditto, related to 1373161

Everything was running fine in 52.1.1, updated to 52.2.0 and Gmail IMAP was not happy!

On one of my Gmail accounts, all folders except Inbox were not showing up, and a new folder named [Gmail]^Junk was created for most of my Gmail accounts.

It also made a real mess of the AppData ImapMail folder as well; it was creating numerous iterations of each folder and file, such as [Gmail].sbd [Gmail-1].sbd [Gmail-2].sbd etc.

Reverted to 52.1.1 and all was well again.

Comment 2

[Wayne Mery (:wsmwk)]

Do you also use check_all_folders_for_new ?

And if so, why do you think you need it?

Comment 3

[Ache]

(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #2)
> Do you also use check_all_folders_for_new ?
> 
> And if so, why do you think you need it?

I don't touch or check this variable.
As I see now in 52.1.1 mail.server.default.check_all_folders_for_new is false and it is its default value.

Comment 4

[Wayne Mery (:wsmwk)]

Also, what are your crash IDs?
See https://support.mozilla.org/en-US/kb/mozilla-crash-reporter#w_viewing-crash-reports

Comment 5

[Ache]

(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #4)
> Also, what are your crash IDs?
> See
> https://support.mozilla.org/en-US/kb/mozilla-crash-reporter#w_viewing-crash-
> reports

bp-439c28f3-462d-4ef1-b157-096c30170616
	6/16/17	3:48 AM
bp-a133d661-0cfa-4583-b39f-26c860170616
	6/16/17	3:29 AM
bp-5c6cd8a0-7658-401a-a69b-6d8a50170616
	6/16/17	3:28 AM
bp-8317c4d0-c1d4-4253-a59f-352dd0170616
	6/16/17	3:14 AM
bp-9590ef01-de55-404f-856c-e595c0170615
	6/16/17	2:52 AM
bp-16e04b45-f2eb-47bc-94fe-58a290170615
	6/16/17	2:52 AM

Comment 6

[Wayne Mery (:wsmwk)]

bp-439c28f3-462d-4ef1-b157-096c30170616 is  nsACString_internal::Assign | nsACString_internal::Assign | nsImapProtocol::CreatePossibleTrashName

But most of your crashes are other signatures...

 CLayeredObject<T>::CContainedObject::Release  bp-a133d661-0cfa-4583-b39f-26c860170616 bp-16e04b45-f2eb-47bc-94fe-58a290170615
 CCreateShaderResourceViewValidator::Validate  bp-5c6cd8a0-7658-401a-a69b-6d8a50170616
 nhasusphoebusosd.dll@0x5502    bp-8317c4d0-c1d4-4253-a59f-352dd0170616

Comment 7

[Jorg K (CEST = GMT+2)]

Only the nsACString_internal::Assign is actionable from our side. The other two are graphics problems and by the looks of it nhasusphoebusosd.dll@0x5502 is also in that area.

Comment 8

[Wayne Mery (:wsmwk)]

(adding some gmail heavyweights)

Comment 9

[Ache]

nhasusphoebusosd.dll is part of ASUS sound driver and not cause any problem for years even with TB hardware acceleration.
I turn off TB hardware acceleration now to be on safe side.

Comment 10

[Wayne Mery (:wsmwk)]

The crashes start at 54.0b1, which is where bug 1176399 would have first appeared

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: 1373493-fix-crash.patch (v1)

This will fix the crash.

In the crash case this is called from nsImapProtocol.cpp:7480:
nsCString trashName(CreatePossibleTrashName(ns->GetPrefix()));

However, there is no certainty that ns->GetPrefix() returns non-null, since a few lines we have:
const char *personalDir = ns ? ns->GetPrefix() : 0;
followed by:
if (personalDir)
So obviously the prefix returned in not guaranteed to be non-null.

Comment 12

[Jorg K (CEST = GMT+2)]

Hmm, or maybe this patch is wrong and we should do this at line 7608:
-    if (!trashFolderExists && GetDeleteIsMoveToTrash() && ns)
+    if (!trashFolderExists && GetDeleteIsMoveToTrash() && ns && ns->GetPrefix())
or since personalDir is set above
+    if (!trashFolderExists && GetDeleteIsMoveToTrash() && personalDir)

Comment 13

[Ache]

Also it don't check that IMAP folder [Gmail] really exists and try to make [Gmail]/Junk per settings which is converted to [Gmail]^Junk. Even if [Gmail]^Junk label is removed from google, I can't remove it from TB in that state. The workaround is to remove [Gmail]^Junk.msf file.

Comment 14

[newsletters.kv]

Experienced the same issues as [e-mail] reported: Gmail-folder structure collapsed, no subfolders visible. For the next 10 minutes and exceeding, TB starts downloading all emails again from IMAP. TB did not crash but uses significant amount of CPU (20%) for redownloading. 

Looking for user-friendly solution here. As [e-mail] wrote: I want it to simply work like version 52.1.1.

Comment 15

[newsletters.kv]

(In reply to newsletters.kv from comment #14)
> Experienced the same issues as [e-mail] reported: Gmail-folder
> structure collapsed, no subfolders visible. For the next 10 minutes and
> exceeding, TB starts downloading all emails again from IMAP. TB did not
> crash but uses significant amount of CPU (20%) for redownloading. 
> 
> Looking for user-friendly solution here. As [e-mail] wrote: I want
> it to simply work like version 52.1.1.

Reinstalled previous TB version 52.1.1. No issues here. Will skip 52.2.0 installation and wait for renewed update.

Comment 16

[:aceman]

(In reply to Jorg K (GMT+2) from comment #11)
> const char *personalDir = ns ? ns->GetPrefix() : 0;

Yes, this is at multiple places. Is it right to assign 0 to a pointer or would nullptr be better?

> followed by:
> if (personalDir)
> So obviously the prefix returned in not guaranteed to be non-null.

I wonder why nsCString returnTrash(prefix) would crash if prefix is null. It could still create an empty string. Are you sure this crashes?

Comment 17

[:aceman]

Reporters, can you start TB with the -jsconsole command line argument so that Error console get opened? Then please check if there are any errors and paste them here.

Comment 18

[Jorg K (CEST = GMT+2)]

Comment on attachment 8878547 [details] [diff] [review]
1373493-fix-crash.patch (v1)

(In reply to :aceman from comment #16)
> Yes, this is at multiple places. Is it right to assign 0 to a pointer or
> would nullptr be better?
Sure, but 0 works, too.

> I wonder why nsCString returnTrash(prefix) would crash if prefix is null. It
> could still create an empty string. Are you sure this crashes?
You're right, it doesn't crash. That just makes it harder. GetPrefix() just returns the namespace member variable m_prefix, as you can see here:
https://dxr.mozilla.org/comm-central/search?q=m_prefix&redirect=false

So if we get an access violation on access to that thing, we really have a problem. Something must have overwritten it.

Comment 19

[Ku]

Experienced the same issues as Ache and newsletters.kv reported: Gmail-folder structure collapsed, no subfolders visible. For the next 10 minutes and exceeding, TB starts downloading all emails again from IMAP. TB did not crash but uses significant amount of CPU (20%) for redownloading. 

Looking for user-friendly solution here. As Ache wrote: I want it to simply work like version 52.1.1.

Reinstalled previous TB version 52.1.1. No issues here.

Comment 20

[Wayne Mery (:wsmwk)]

see bug 557252 comment 2

Comment 21

[Wayne Mery (:wsmwk)]

Ache, Ku, u597066 ...

(In reply to :aceman from comment #17)
> Reporters, can you start TB with the -jsconsole command line argument so
> that Error console get opened? Then please check if there are any errors and
> paste them here.

Comment 22

[Ache]

(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #21)
> Ache, Ku, u597066 ...
> 
> (In reply to :aceman from comment #17)
> > Reporters, can you start TB with the -jsconsole command line argument so
> > that Error console get opened? Then please check if there are any errors and
> > paste them here.

I still got from imap "Unknown command jsd1233eo23" but it is not reflected on -jconsole otput:

IndexedDB UnknownErr: ActorsParent.cpp:599  (unknown)
undefined  Promise-backend.js:935
** Preference parsing warning (line 2) = preserving unexpected JS escape sequence **

JavaScript 1.6's for-each-in loops are deprecated; consider using ES6 for-of instead[Learn More]  commons.js:184:8
Use of nsIDOMWindowInternal is deprecated. Use nsIDOMWindow instead.  bootstrap.js:492:20
IndexedDB UnknownErr: ActorsParent.cpp:599  (unknown)
UnknownError  indexed-db.js:59:9
undefined  Promise-backend.js:935
UnknownError  indexed-db.js:59:9
No chrome package registered for chrome://folderaccount/locale/folderAccount.propertiesJavaScript 1.6's for-each-in loops are deprecated; consider using ES6 for-of instead[Learn More]  prefs.js:156:8
XML Parsing Error: undefined entity
Location: chrome://rightencoding/content/rightencoding_overlayTB.xul
Line Number 23, Column 3:  rightencoding_overlayTB.xul:23:3
Use of Mutation Events is deprecated. Use MutationObserver instead.  calendar-widgets.xml:506:18
TypeError: tab is undefined[Learn More] tabmail.xml:1070:15
JavaScript 1.6's for-each-in loops are deprecated; consider using ES6 for-of instead[Learn More]  ctypes-utils.jsm:119:8
14:19:1.125 WARN firetray.Handler Chat not supported for this environment. Chat not loaded
NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN: Cannot modify properties of a WrappedNative folderPane.js:2185
NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN: Cannot modify properties of a WrappedNative folderPane.js:2185
2017-06-21 14:19:08	gloda.indexer	WARN	Problem during [job:folder id:457 items:0 offset:0 goal:null], bailing: Error: Got impossible folder ID: 457
TypeError: parent is null[Learn More]  folderPane.js:26:12

Comment 23

[Ache]

(In reply to Ache from comment #22)
> (In reply to Wayne Mery (:wsmwk, NI for questions) from comment #21)
> > Ache, Ku, u597066 ...
> > 
> > (In reply to :aceman from comment #17)
> > > Reporters, can you start TB with the -jsconsole command line argument so

BTW, probability of folders corruption is essentially lower with -jsconsole arg for the reason unknown, perhaps some race. It still happens anyway after few starts done. More starts - more Gmail accs becomes damages. All folders are redownloaded in any case and columns order in the subjects pane is resetted.

Comment 24

[Wayne Mery (:wsmwk)]

There does seem to be some twilight zone factor involved.

Questions:
How many gmail accounts do you have defined to Thunderbird?
Are any set as the default account?

Comment 25

[Ache]

(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #24)
> There does seem to be some twilight zone factor involved.
> 
> Questions:
> How many gmail accounts do you have defined to Thunderbird?
> Are any set as the default account?

3 accounts with one default from them.

Comment 26

[Jorg K (CEST = GMT+2)]

All people affected, please try TB 52.2.1: http://ftp.mozilla.org/pub/thunderbird/candidates/52.2.1-candidates/build1/

Comment 27

[Ache]

52.2.1 works fine so far(In reply to Jorg K (GMT+2) from comment #26)
> All people affected, please try TB 52.2.1:
> http://ftp.mozilla.org/pub/thunderbird/candidates/52.2.1-candidates/build1/

works fine so far

Comment 28

[:aceman]

So what was changed in 52.2.1?

Comment 29

[Wayne Mery (:wsmwk)]

https://hg.mozilla.org/releases/comm-esr52
9087360f81b0 2017-06-21 22:36 +0200	Jorg K - Backed out changeset 819e836ed566 (bug 682474) for causing Gmail problems. a=jorgk
762c13339d3d 2017-06-21 22:35 +0200	Jorg K - Backed out changeset 2e1b58d59899 (bug 1359967) for causing Gmail problems. a=jorgk
3876b639b13d 2017-06-21 22:23 +0200	Jorg K - Backed out changeset fdb3acad393d (bug 1176399) for causing Gmail problems. a=jorgk IGNORE IDL

Comment 30

[Jorg K (CEST = GMT+2)]

Fixed by backouts from TB 52 and later. Open report remains: Bug 1379475.