Open Bug 531929 Opened 15 years ago Updated 5 years ago

folder pane context-menu command events may be executed after folder has changed; dangerous commands like empty junk may end up with the wrong target

Categories

(Thunderbird :: Folder and Message Lists, defect)

Product:
Thunderbird
Component:
Folder and Message Lists
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

(blocking-thunderbird3.1 needed)

Tracking Flags:
Tracking Status
blocking-thunderbird3.1 --- needed

People

(Reporter: macros, Unassigned)

Details

(Keywords: dataloss, perf) 

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.5) Gecko/20091109 Ubuntu/9.10 (karmic) Firefox/3.5.5
Build Identifier: Thunderbird 3 RC

From the context menu I chose "Empty Junk" then changed the selected folder to my Inbox. My inbox was emptied, not the Junk folder. Also happened with choosing Empty Trash.

My Thunderbird is going very slowly as the indexing is running, and I have noticed other similar delayed actions.

Eg. I chose Important from the Tags context menu on a mail item, then changed to the next email, and the second email was tagged as important, not the first.

Reproducible: Always

Steps to Reproduce:
1. With indexing running (or a slow machine, I'd say) choose Empty Junk or Empty Trash;
2. Quickly change to Inbox;
3. Contents of my Inbox was deleted.
Actual Results:  
Inbox contents deleted.

Expected Results:  
Empty the junk folder
Version: unspecified → 3.0
To add: this is for a POP3 account.
(In reply to comment #0)
> User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.5)
> Gecko/20091109 Ubuntu/9.10 (karmic) Firefox/3.5.5
> Build Identifier: Thunderbird 3 RC
> 
> From the context menu I chose "Empty Junk" then changed the selected folder to
> my Inbox. My inbox was emptied, not the Junk folder. Also happened with
> choosing Empty Trash.

Does this happens in safe-mode (http://kb.mozillazine.org/Safe_mode)? Did you notice any error messages in Tools -> Error console ?

> My Thunderbird is going very slowly as the indexing is running, and I have
> noticed other similar delayed actions.

We have a policy of one issue per bug - can you file a new bug for that ?
 
> Eg. I chose Important from the Tags context menu on a mail item, then changed
> to the next email, and the second email was tagged as important, not the first.

Can you file a new issue for that (search we might already have issues with the same context) ?
 
> Reproducible: Always
> 
> Steps to Reproduce:
> 1. With indexing running (or a slow machine, I'd say) choose Empty Junk or
> Empty Trash;

Where do you choose empty Junk or Emptyr trash ? from a menu item, from a context menu ?
Keywords: dataloss
Cameron, you use extension "Delete Junk Context Menu" (0.3.2.1)? I suspect that this issue is a dupe of bug #524247.
(In reply to comment #2)
> 
> Does this happens in safe-mode (http://kb.mozillazine.org/Safe_mode)? Did you
> notice any error messages in Tools -> Error console ?

I will try it again, and check the error console as well.

> Can you file a new issue for that (search we might already have issues with the
> same context) ?

Will do

> Where do you choose empty Junk or Emptyr trash ? from a menu item, from a
> context menu ?

I chose it from the context menu on the Junk folder.
(In reply to comment #3)
> Cameron, you use extension "Delete Junk Context Menu" (0.3.2.1)? I suspect that
> this issue is a dupe of bug #524247.

I have the extension installed, but it's disabled (left overs from TB2). I only have "Empty Junk" on the Junk folder context menu.
(In reply to comment #5)
> I have the extension installed, but it's disabled (left overs from TB2). I only
> have "Empty Junk" on the Junk folder context menu.
Try also after uninstall extension  "Delete Junk Context Menu". If you don't have success, try in safe-mode also as suggest in comment #2.
Post a feedback here.
OK, I've uninstalled it, and it still happened.

I've just done it in safe mode, and it still happened!
Cameron you see anythings in Error console?
...and you have options-->advanced "Enabled Global Search and Indexer" set to true?
(In reply to comment #8)
> Cameron you see anythings in Error console?

Sorry, forgot to check the error console the first time, so replicated it again.

Error: msgHdr is null
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 862

(In reply to comment #9)
> ...and you have options-->advanced "Enabled Global Search and Indexer" set to
> true?

Yes, global search is enabled.
You can confirm that this problem is only when gloda indexing is running?

Using Account Settings window you can see the path of mail for account that has this problem. If you goto in this path and delete file Inbox.msf (*Warning* only Inbox.msf and not Inbox) this solve your issue?
(In reply to comment #11)
> You can confirm that this problem is only when gloda indexing is running?

With gloda indexing disabled I am unable to replicate it, as I don't have enough time between clicking "Empty Junk" in the context menu and then click the folder.

> Using Account Settings window you can see the path of mail for account that has
> this problem. If you goto in this path and delete file Inbox.msf (*Warning*
> only Inbox.msf and not Inbox) this solve your issue?

I've actually been using a test folder with a dozen or so emails in it. I created the folder just for the testing, so not sure if deleting the msf will make much difference. Let me know if you want me to try that.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: perf
(In reply to comment #12)
> Let me know if you want me to try that.
Yes, delete Inbox.msf.
(In reply to comment #13)
> Yes, delete Inbox.msf.

OK, running Thunderbird in safe-mode after deleting inbox.msf, I was able to replicate the bug again.

To be explicit:

1) Chose "Empty Junk" from the context menu on the junk folder of my local account (POP3);
2) After clicking "Empty Junk" quickly changed to inbox;
3) Inbox content is deleted. The size of the Inbox file is not modified though.
(In reply to comment #14)
> 3) Inbox content is deleted. The size of the Inbox file is not modified though.
Then when you close TB and delete Inbox.msf when restart TB Inbox is restored with all mails that are "deleted"?
(In reply to comment #15)
> Then when you close TB and delete Inbox.msf when restart TB Inbox is restored
> with all mails that are "deleted"?

Yes, it seems so.
Summary: Mail deleted from wrong folder using "Empty Junk" → Empty Junk while GLODA indexing causes corruption of Inbox.msf file: messages disappear from the folder
Component: Folder and Message Lists → Database
Product: Thunderbird → MailNews Core
QA Contact: folders-message-lists → database
Version: 3.0 → Trunk
When the inbox contents appear deleted, if you switch to another folder and then switch back to the inbox, do the inbox contents reappear (without deleting the .msf)?  If you quit Thunderbird and restart and re-select the inbox, do the contents reappear?

The code that triggers trash/junk emptying seems incapable of doing the wrong thing, and I really don't see how the msf could get corrupted, which suggests a folder display problem rather than anything more sinister.
(In reply to comment #17)
> When the inbox contents appear deleted, if you switch to another folder and
> then switch back to the inbox, do the inbox contents reappear (without deleting
> the .msf)?  If you quit Thunderbird and restart and re-select the inbox, do the
> contents reappear?

Switching doesn't show the contents, neither does restarting TB.
Cameron, is this a smart folder inbox, or a normal inbox?  (In other words, does the top of the folder pane say "Smart Folders" or "All Folders"?)

bienvenu, thoughts on how the emptyTrash/emptyJunk functions could cause an apparent problem such as this?

I'm still inclined to believe it's a FolderDisplay problem.  Perhaps we are propagating/doing something bad to the DBFolderInfo somehow that causes this.

If it's a smart folder, I could perhaps see the initiation of a delete that gets delivered to the folder display when the delete completes causing some form of trouble...
asuth, could it be that the event queue is so full of gloda indexing stuff that the menu command is processed after the folder selection is changed? I.e., the user picks empty trash, but before that's translated into a call into folderPane.js, the folder selection event gets processed? Cameron, do you see the confirmation alert or have you turned that off? We do get the selected folder before the confirmation prompt is put up, so it can't be that the confirmation prompt is pumping events.  None of this seems likely since all this is supposed to be synchronous and modal, afaik.
I'm not aware any way for the event sequence of "user selects empty trash", "user selects another folder" to get reordered, which is the only way the emptyTrash/emptyJunk methods could do something incorrect themselves.

However, it does seem like gloda being busy could draw out an asynchronous operation.  Deletion's notification is async and likely the trash-related dues are async which is why I accuse them in the library with the letter opener.

Another unsubstantiated possibility would be that the active gloda indexing job somehow goes rogue because of the trash/junk emptying.  Certainly deleting the inbox .msf would give the gloda indexing sweep a lot of work to do, and if that was the problem, the attempt to reproduce would be likely to cause that same situation to arise.  However, the fact that this happens when emptying the trash (which gloda won't go in unless there are nested folders) suggests it's not gloda related.

Cameron... is my assumption that you don't have nested folders in the trash when you emptied the trash true?
local trash emptying is synchronous, even if there are sub-folders.
(In reply to comment #19)
> Cameron, is this a smart folder inbox, or a normal inbox?  (In other words,
> does the top of the folder pane say "Smart Folders" or "All Folders"?)

Normal inbox.

(In reply to comment #21)
> Cameron... is my assumption that you don't have nested folders in the trash
> when you emptied the trash true?

Most of the replication I've done has been using the Junk folder, but the one time I did it via trash there were no folders in it, just mail.
(In reply to comment #20)
> Cameron, do you see
> the confirmation alert or have you turned that off? We do get the selected
> folder before the confirmation prompt is put up, so it can't be that the
> confirmation prompt is pumping events.  None of this seems likely since all
> this is supposed to be synchronous and modal, afaik.

I've got it turned off, so don't see any messages.
Is this really a confirmed (on 12-03) bug?  I dont' see that anyone else was able to reproduce.
reporter writes: "I haven't seen it since my index was built." so I don't see why we'd still want this open
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WORKSFORME
I'm now on TB 3.1, and just lost all the contents of another folder doing the above. And by lost I mean the actual folder contents are gone this time, not just the index being empty.

I deleted the .msf and did NOT recover the files. The non-msf file is 0 bytes.

In this case, for some reason, TB 3.1 is choosing to index a folder that has over 10,000 emails in it, and my CPU is sitting pretty consistently at about 100%.

To repeat what I've put above, here is my actions.

I right-clicked on the junk folder, chose "Empty Junk" in the context menu, then clicked another folder to view an email, at which time the folder I had clicked on was emptied.

I know people have said that it isn't possible above, but somehow it has happened to me again. If someone wants to talk to me in real time I can try to help narrow this down. It's something that is frustrating me, as I'm losing mail and that's not really acceptable!
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
The XUL menu click handler must either be delaying its dispatch or whatever it dispatches it to must be re-dispatching (because it needs to "eval" the oncommand block?).  The onpuphiding logic may be contributing to this problem through its attempt to restore the transient selection.

All the context-menu triggered folder pane code is accordingly pretty dangerous.  The safe/correct way for right-click handling code to work is to save a reference to what it was pointing at when the right-click originated the menu happens and makes sure that it gets passed to the thing being invoked.  Regrettably, the platform does not make this easy and the emergent effect is like it is trying to actively stab us in the back.

What is probably the simplest way to address this serious problem that will have the desired net effect is to cause fillFolderPaneContextMenu() in mailContextMenus to explicitly save the state of the selection it is using in gFolderTreeController.  All operations in the folder tree controller that operate inside an implied context-selection triggering would retrieve their context from there.

Since it is clear that the event sequence is already capable of betraying us, the following additional safe-guard would be applied: We explode if we go to set the context and it is already set.  We explode if we go to retrieve the context and it is not set.  We clear the context when we successfully retrieve it.  When we explode we a) set a flag that causes all subsequent attempts to ever do anything with the context to explode, b) pop up a warning to the user that something horrible has happened and request that they quit thunderbird.

The probability of the exploding case happening should be much more rare than the hazard condition that currently exists.  Since the exploding case is a situation where we would have, with high probability, acted on the wrong thing, this seems generally like an improvement.

Another alternative is to try and store the selected folders on the popped-up menu and propagate the state through to the invoked command.  Given that there is only ever one of those popup menus, I think it reduces to the exact same problem though (unless we dynamically create new popup menus, which might not be a bad idea.)

Whoever fixes this can also try and solve the problem by attempting to address the fundamental platform and event dispatch horrors.  I am going to try and guilt Blake into doing this since Standard8 is currently unavailable.  I am willing to be the reviewer.
blocking-thunderbird3.1: --- → .1+
Component: Database → Folder and Message Lists
OS: Windows Vista → All
Product: MailNews Core → Thunderbird
QA Contact: database → folders-message-lists
Hardware: x86 → All
Summary: Empty Junk while GLODA indexing causes corruption of Inbox.msf file: messages disappear from the folder → folder pane context-menu command events may be executed after folder has changed; dangerous commands like empty junk may end up with the wrong target
Oh, and I marked this blocking because this is obviously a very bad problem and some brief poking/testing by me suggests that the event sequence is more suspect than originally believed.

This could hopefully be tested by generating a synthetic click event on another folder immediately after generating the first click event on the context menu to trigger the dubious action.  Ideally this test would be written before the fix to validate that our suspicions are totally in fact correct.
(In reply to comment #27)
> I'm now on TB 3.1, and just lost all the contents of another folder doing the
> above. And by lost I mean the actual folder contents are gone this time, not
> just the index being empty.
> 
> I deleted the .msf and did NOT recover the files. The non-msf file is 0 bytes.

Er, forgot to add the part to my message where I convey my apologies that this happened (again :( and that we did not prioritize this bug higher earlier.  There can be a lot of potentially interacting factors that we were trying to rule out, but analysis petered out and without additional confirmations of the bug did not get revisited.  (A lot of times very rogue anti-viruses turn out to be the problem...)

Many thanks for having a cool head in your reports of this problem; this bug sucks and is no doubt very frustrating to experience, so I appreciate your bug reports keeping things friendly.
(Whoops, thought I added blake but something must have gone wrong; thanks for adding him Ludo.  Blake, I am trying to smoothtalk you into doing the fix for this (and then I, generous man that I am, would do the review).  Please see above.)
Wow, 7 months later & someone finally believes me! :)

Thanks Andrew, hope it can be sorted. I can probably trigger it again if I can make TB rebuild the GLODA, as that seems to slow my laptop to a crawl.
OK, a little more info regarding the data loss:

I managed to trigger it again on my Inbox by accident last night... and the inbox data file was still large (112MB or so), but deleting the msf didn't recover the emails. It now looks like the compact has automatically been triggered & it's down to 108KB. 

Luckily I'd backed up the file a few days ago, so I shouldn't lose any email once I copy out the emails I want to keep and the GLODA has finished rebuilding.
It doesn't look like we're going to get a resolution on this for 3.1.1. Therefore moving to 3.1.2 and assigning to Andrew as he seems to know most about what is going on here.
Assignee: nobody → bugmail
Status: REOPENED → NEW
blocking-thunderbird3.1: .1+ → .2+
blocking-thunderbird3.1: .2+ → .3+
Assignee: bugmail → nobody
blocking-thunderbird3.1: .3+ → needed
Keywords: helpwanted

(in the wake of bug 1364167) see comment 27 and 28. I haven't tried this to see if it still exists. Perhaps the code has changed?

Flags: needinfo?(geoff)
Keywords: helpwanted

I think this has been mitigated in two ways by bug 1018960:

  • you can't empty junk from a non-junk folder
  • the confirmation dialog shows the name of the folder in its title (although you can stop the dialog showing)
Flags: needinfo?(geoff)
You need to log in before you can comment on or make changes to this bug.