Permanent Orange: TEST-UNEXPECTED-FAIL | test_index_messages_imap_offline.js | test_index_messages_imap_online.js | test_index_messages_imap_online_to_offline.js | Error console says ... failure code: 0x80550007 [nsIMsgFolder.getStringProperty]

RESOLVED FIXED in Thunderbird 26.0

Status

MailNews Core
Database
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: standard8, Assigned: hiro)

Tracking

({intermittent-failure})

Trunk
Thunderbird 26.0
intermittent-failure
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird24 fixed, thunderbird25 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

4 years ago
Once bug 901514 is fixed, there's then a few more gloda failures of the kind:

TEST-INFO | (xpcshell/head.js) | test _async_driver finished (2)
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80500001: file /Users/moztest/comm/main/src/mozilla/intl/uconv/src/nsCharsetConverterManager.cpp, line 300
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80500001: file /Users/moztest/comm/main/src/mozilla/intl/uconv/src/nsCharsetConverterManager.cpp, line 300
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80500001: file /Users/moztest/comm/main/src/mozilla/intl/uconv/src/nsCharsetConverterManager.cpp, line 300
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80500001: file /Users/moztest/comm/main/src/mozilla/intl/uconv/src/nsCharsetConverterManager.cpp, line 300
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80500001: file /Users/moztest/comm/main/src/mozilla/intl/uconv/src/nsCharsetConverterManager.cpp, line 300
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80500001: file /Users/moztest/comm/main/src/mozilla/intl/uconv/src/nsCharsetConverterManager.cpp, line 300
2013-08-06 16:28:15	gloda.datastore	DEBUG	!! mapped 34 from imap://user@localhost/gabba27/gabba28
2013-08-06 16:28:15	gloda.index_msg	DEBUG	folderDeleted notification
2013-08-06 16:28:15	gloda.index_msg	INFO	Processing deletion of folder gabba28.
System JS : ERROR resource:///modules/gloda/datastore.js:2072
                     Component returned failure code: 0x80550007 [nsIMsgFolder.getStringProperty]

TEST-UNEXPECTED-FAIL | ../../../../resources/logHelper.js | Error console says [stackFrame Component returned failure code: 0x80550007 [nsIMsgFolder.getStringProperty]] - See following stack:
JS frame :: ../../../../resources/logHelper.js :: _errorConsoleTunnel.observe :: line 63
native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

TEST-INFO | (xpcshell/head.js) | exiting test
2013-08-06 16:28:15	gloda.index_msg	DEBUG	folderMoveCopy notification (Move: true)
System JS : ERROR resource:///modules/gloda/datastore.js:2072
                     Component returned failure code: 0x80550007 [nsIMsgFolder.getStringProperty]

TEST-INFO | (xpcshell/head.js) | test _async_driver pending (2)

TEST-UNEXPECTED-FAIL | ../../../../resources/logHelper.js | Error console says [stackFrame Component returned failure code: 0x80550007 [nsIMsgFolder.getStringProperty]] - See following stack:
JS frame :: ../../../../resources/logHelper.js :: _errorConsoleTunnel.observe :: line 63
native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

TEST-INFO | (xpcshell/head.js) | exiting test
(Reporter)

Comment 1

4 years ago
Created attachment 786318 [details] [diff] [review]
Proposed fix

I'm not 100% sure about this. We seem to be receiving a folder deletion, and then a rename for the said folder - however, the folder properties don't exist, as it was deleted, so we blow up.

This catches the blowing up, I couldn't spot what was going on with the delete versus rename stuff.
Attachment #786318 - Flags: review?(Pidgeot18)
(Reporter)

Updated

4 years ago
Summary: Permanent Orange: TEST-UNEXPECTED-FAIL | logHelper.js | Error console says ... failure code: 0x80550007 [nsIMsgFolder.getStringProperty] → Permanent Orange: TEST-UNEXPECTED-FAIL | test_index_messages_imap_offline.js, test_index_messages_imap_online.js, test_index_messages_imap_online_to_offline.js | Error console says ... failure code: 0x80550007 [nsIMsgFolder.getStringProperty]
(Reporter)

Updated

4 years ago
Keywords: intermittent-failure
(Reporter)

Updated

4 years ago
Summary: Permanent Orange: TEST-UNEXPECTED-FAIL | test_index_messages_imap_offline.js, test_index_messages_imap_online.js, test_index_messages_imap_online_to_offline.js | Error console says ... failure code: 0x80550007 [nsIMsgFolder.getStringProperty] → Permanent Orange: TEST-UNEXPECTED-FAIL | test_index_messages_imap_offline.js | test_index_messages_imap_online.js | test_index_messages_imap_online_to_offline.js | Error console says ... failure code: 0x80550007 [nsIMsgFolder.getStringProperty]
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 42

4 years ago
(In reply to Mark Banner (:standard8) from comment #1)

> I'm not 100% sure about this. We seem to be receiving a folder deletion, and
> then a rename for the said folder - however, the folder properties don't
> exist, as it was deleted, so we blow up.
> 
> This catches the blowing up, I couldn't spot what was going on with the
> delete versus rename stuff.

Renaming IMAP folder does not preserve .msf file. It just removes old .msf file in nsImapMailFolder::RenameLocal. I do not know why old .msf file is not renamed as well.
(Assignee)

Comment 43

4 years ago
Found the reason. bug 56044
(Assignee)

Comment 44

4 years ago
My comment in #42 was not problem here.

The problem here is nsMsgDBFolder holds old mPath after renaming.
In this failure case, the path was /tmp/xpcshell/xpcshellprofile/ImapMail/localhost/gabba28.msf. It has to be
/tmp/xpcshell/xpcshellprofile/ImapMail/localhost/gabba27.sbd/gabba28.msf.
(Assignee)

Comment 45

4 years ago
(In reply to Hiroyuki Ikezoe (:hiro) from comment #44)

> The problem here is nsMsgDBFolder holds old mPath after renaming.
> In this failure case, the path was
> /tmp/xpcshell/xpcshellprofile/ImapMail/localhost/gabba28.msf. It has to be
> /tmp/xpcshell/xpcshellprofile/ImapMail/localhost/gabba27.sbd/gabba28.msf.

I was wrong. /tmp/xpcshell/xpcshellprofile/ImapMail/localhost/gabba28.msf is correct here to getStringProperty("indexingPriority"). It is called for old folder.
So the real problem is that gloda_ds_renameFolder is called after renaming process has done. As the result, gabba28.msf of old folder was removed.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 49

4 years ago
Created attachment 790645 [details] [diff] [review]
Do not remove .msf files before folderRenamed notifier

This patch consists two parts.

a) Invoke PropagateDelete() followed by NotifyFolderRenamed() in nsImapMailFolder::RenameClient().
b) Check that oldFolder sits in this._folderByURI in gloda_ds_renameFolder().

About a)
PropagateDelete() invokes 'folderDeleted' in gloda/modules/index_msg.js, it ends up removing the folder URI in _folderByURI. These processes have to be done after gloda_ds_renameFolder().

About b)
In case of moving folder for IMAP, gloda_ds_renameFolder is called twice. The first one comes from NotifyFolderRenamed() in RenameClient. The second one comes from folderMoveCopyCompleted. The second one should be ignored.
Attachment #790645 - Flags: feedback?(mbanner)
(Assignee)

Updated

4 years ago
Attachment #790645 - Flags: feedback?(Pidgeot18)
Comment hidden (Treeherder Robot)
Comment on attachment 786318 [details] [diff] [review]
Proposed fix

The fact that _mapFolder is exploding suggests that we don't want to be mapping the folder at all.  Assuming Hiro's patch makes us not call _mapFolder on the dead URI, I think that's exactly what we want.
Attachment #786318 - Flags: feedback-
Comment on attachment 790645 [details] [diff] [review]
Do not remove .msf files before folderRenamed notifier

The added gloda guard seems fine and good if it helps us avoid calling _mapFolder on a dead folder and otherwise avoid doing things we don't want to do.

My non-binding opinion on moving the C++ line is that we should really add a comment that explains why it should be where it is.  Since we're moving it, the position arguably does matter.

For example:
// Do not propagate the deletion until after we have (synchronously) notified
// all listeners about the rename.  This allows them to access properties on
// the source folder without experiencing failures.
Attachment #790645 - Flags: feedback+
(Reporter)

Updated

4 years ago
Attachment #786318 - Attachment is obsolete: true
Attachment #786318 - Flags: review?(Pidgeot18)
(Reporter)

Comment 53

4 years ago
Comment on attachment 790645 [details] [diff] [review]
Do not remove .msf files before folderRenamed notifier

Thanks to Andrew for his comments. Given them, I'm definitely happy with this as well, so r=Standard8 with the comment added.
Attachment #790645 - Flags: feedback?(mbanner)
Attachment #790645 - Flags: feedback?(Pidgeot18)
Attachment #790645 - Flags: feedback+
(Reporter)

Comment 54

4 years ago
Comment on attachment 790645 [details] [diff] [review]
Do not remove .msf files before folderRenamed notifier

[Triage Comment]
The completed patch has a=Standard8 to land on aurora and beta as well as we need it there.
Attachment #790645 - Flags: approval-comm-beta+
Attachment #790645 - Flags: approval-comm-aurora+
(Assignee)

Comment 55

4 years ago
Created attachment 791078 [details] [diff] [review]
Do not remove .msf files before folderRenamed notifier V2

Carrying over review+.

Add the comment in comment 52.
Assignee: mbanner → hiikezoe
Attachment #790645 - Attachment is obsolete: true
Attachment #791078 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Comment hidden (Treeherder Robot)
https://hg.mozilla.org/comm-central/rev/b55f70dce418
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 26.0
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Reporter)

Comment 62

4 years ago
Comment on attachment 791078 [details] [diff] [review]
Do not remove .msf files before folderRenamed notifier V2

[Triage Comment]
We need this on aurora/beta to fix unit test bustage. a=Standard8
Attachment #791078 - Flags: approval-comm-beta+
Attachment #791078 - Flags: approval-comm-aurora+
(Reporter)

Comment 63

4 years ago
https://hg.mozilla.org/releases/comm-aurora/rev/5e2a78f0636b
https://hg.mozilla.org/releases/comm-beta/rev/bd07b0c01be1
status-thunderbird24: --- → fixed
status-thunderbird25: --- → fixed
Depends on: 916095
You need to log in before you can comment on or make changes to this bug.