636210 - Hang when quitting SeaMonkey if "Always clear my private data when I close SeaMonkey" is combined with "Ask me before clearing private data"

Status: VERIFIED FIXED

(SeaMonkey :: General, defect)

Description

[Michal Urban]

User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b11) Gecko/20110209 Firefox/ SeaMonkey/2.1b2
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b11) Gecko/20110209 Firefox/ SeaMonkey/2.1b2

With "Always clear my private data when I close SeaMonkey" preference checked, SM 2.1b2 crashes upon closing. When restarted, clear data dialog appears again and works when clicked.

Reproducible: Always

Steps to Reproduce:
1. Check "Always clear my private data when I close SeaMonkey" in preferences
2. Check also "Ask me before clearing private data"
3. Close SeaMonkey
Actual Results:  
Browser hangs with the clear data dialog unresponsive.

Expected Results:  
Clear data dialog should respond to clicking an close browser normally.

Comment 1

[Philip Chee]

When SeaMonkey crashes did you allow the crash reporter to send a crash report to Mozilla? If so go to about:crashes. There should be a list of clickable links to the crash reports in most-recent to oldest order.

Comment 2

[Michal Urban]

More testing revealed that the exit hang only occurs when the "Ask me before clearing private data" box is checked. When it's clear, browser quits normally.

Regarding crash log, here is what happens:

Upon quitting, only Clear Private Data dialog remains and is unresponsive (beach ball, also see screen shot). There is no complete crash which would invoke Mozilla crash reporter.

When I Force Quit SeaMonkey, Apple's crash reporter kicks in and shows info attached in a log file.

about:crashes is empty. Is there a way of enabling it somewhere?

Comment 3

[Michal Urban]

Attachment: SeaMonkey hangs when clearing private data

Comment 4

[Michal Urban]

Attachment: Info from Apple crash reporter

Comment 5

[Philip Chee]

Without symbols from the mozilla symbol server the information from the apple crash reporter is useless.

Alternative ways to get a stacktrace from OSX:

https://developer.mozilla.org/En/How_to_get_a_stacktrace_for_a_bug_report#Mac

Comment 6

[Michal Urban]

I may look into getting stack trace, however I'm wondering if anybody is able to replicate this issue? It should be easy - load SM 2.1beta, check "always clear" and "ask me" in preferences and close the browser...

I'll try to disable add-ons to see whether they interfere with this.

Comment 7

[Michal Urban]

I get the same behaviour with all Extensions disabled.

Comment 8

[Philip Chee]

Open the Task Manager (or whatever it's called in OSX). Close SeaMonkey. Do you see one or more plugin-container processes?

Similar bugs:
Bug 449707 - Selecting "clear private data" freezes browser
" Upon closing browser, dialogue box appears to clear private data. Clicking on "Clear now" freezes the browser (FF3)"

Bug 635090 - Multiple instances of plugin-container.exe spawn when closing Seamonkey and, if installed, launches acrobat.exe

Bug 633427 - Clearing cookies launches instance of plugin-container for each plugin installed.

Comment 9

[Michal Urban]

No plugin processes are listed in Mac's Activity Monitor (or "top" in Terminal), only when I load a page with e.g. Flash plugin, I can see a "SeaMonkey plugin process".

However, this issue occurs even without any plugins loaded, with only a single blank page displayed.

I just replicated this on another machine (MacBook5,1 running OS 10.6.5), so I guess anybody with a Mac/Snow Leopard should get this. Again, it only happens when "Ask me before clearing..." is checked.

"Always clear... when I close..." functionality seems to work fine as long as "Ask me..." is clear.

Comment 10

[Philip Chee]

The "clear plugin private data when clearing cookies" patch made it to SeaMonkey 2.1b2. I'd ask you to try SeaMonkey 2.1b1 but this will wipe your history and bookmarks. If you try to reproduce with 2.1b1, do it with a separate profile or make a backup of your SeaMonkey profile (or at least all your *.sqlite files).

Comment 11

[Michal Urban]

Tested with 2.1b1, using a new profile. Behaviour is the same (hang upon quitting), when "Ask me..." is cleared SeaMonkey closes OK. The only difference I can see is that when I Force Quit hanging 2.1b1, Apple's crash reporter doesn't come up like it did with 2.1b2.

Comment 12

[Philip Chee]

Sorry, I'm out of ideas except for some antivirus software interfering but I heard that Macs don't have viruses or something.

Comment 13

[Michal Urban]

Thanks for your effort, Philip! No antivirus on my Mac.

As I wrote, a developer with a Mac should be able to replicate this easily - it was consistent for me on 2 machines. I found a workaround of having "Always clear" enabled without "Ask me" option acceptable.

Comment 14

[Philip Chee]

Stefan has a Mac, I'll CC him on this bug.

Comment 15

[Stefan [:stefanh]]

I can reproduce this (trunk). Hmm, could it be related to the shutdown process and that we might have reached too far there when the clear pd window appears?

Michael - have you seen this on 2.0.x?

Comment 16

[Michal Urban]

2.0.x builds (currently on 2.0.12) work fine for me. I only encountered this on 2.1 betas that I tried out.

Comment 17

[Stefan [:stefanh]]

OK, thanks. Do you have any idea on when it started to happen? A regressionwindow would be valuable. If you have the time, older builds can be found at http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/. It's probably best to start with the oldest releases/candidates (a1).

Comment 18

[Philip Chee]

Your bookmarks and history will be broken/gone if you switch between various 2.1betas/alphas. I suggest you use a separate throwaway profile.

Comment 19

[Stefan [:stefanh]]

In nsSuiteGlue.js

167-174:
      case "places-shutdown":
        if (this._isPlacesShutdownObserver) {
          Services.obs.removeObserver(this, "places-shutdown");
          this._isPlacesShutdownObserver = false;
        }
        // places-shutdown is fired when the profile is about to disappear.
        this._onProfileShutdown();
        break;


280-285:
  // profile shutdown handler (contains profile cleanup routines)
  _onProfileShutdown: function()
  {
    this._shutdownPlaces();
    Sanitizer.checkAndSanitize();
  },

If I call Sanitizer.checkAndSanitize() when "quit-application-granted" is fired I don't hang.

Comment 20

[Stefan [:stefanh]]

I did some testing and the regression-window is obvious:

Before bug 580658 we ran _onProfileShutdown() (which calls Sanitizer.checkAndSanitize()) when  "quit-application-granted" was fired. After the changes in bug 580658, we're calling _onProfileShutdown() after "places-shutdown" is fired. After bug 580658 we didn't showed the dialog at all (bug 588651), that was fixed in toolkit by bug 580892 2010-08-27.

Builds before bug 580658 (2010-08-08 and earlier) doesn't hang when the dialog show up.
Builds from 2010-08-09 to 2010-08-27 exhibits bug 588651 (naturally)
The nightly from 2010-08-28 hangs when the dialog show up.

Comment 21

[Stefan [:stefanh]]

Michal, no need to look for a regression window - I did some testing/digging:

Before bug 580658 we ran _onProfileShutdown() (which calls Sanitizer.checkAndSanitize()) when  "quit-application-granted" was fired. After the changes in bug 580658, we call _onProfileShutdown() when "places-shutdown" is fired. After bug 580658 we didn't showed the dialog at all (bug 588651), that was fixed in toolkit by bug 580892 2010-08-27.

Builds before bug 580658 (2010-08-08 and earlier) doesn't hang when the dialog show up.
Builds from 2010-08-09 to 2010-08-27 exhibits bug 588651 (naturally)
The nightly from 2010-08-28 hangs when the dialog show up.

Comment 22

[Stefan [:stefanh]]

Attachment: Stack from hang

Here's a stack from the hang

Comment 23

[Michal Urban]

Looks like you've found the problem, I can report this:

SM 2.1 Alpha 2 worked as expected - "Clear" dialog shows up upon exit, no hang/crash.

SM 2.1 Alpha 3 is broken - no dialog on exit, seems to quit without crash though. Dialog appears upon restart.

SM 2.1 Beta 1 and 2 show dialog but hang upon quitting, have to be Force Quit.

Comment 24

[[e-mail]]

(In reply to comment #23)
> Looks like you've found the problem, I can report this:
> 
> SM 2.1 Alpha 2 worked as expected - "Clear" dialog shows up upon exit, no
> hang/crash.
> 
> SM 2.1 Alpha 3 is broken - no dialog on exit, seems to quit without crash
> though. Dialog appears upon restart.
This was bug 588651.

> SM 2.1 Beta 1 and 2 show dialog but hang upon quitting, have to be Force Quit.

Comment 25

[[e-mail]]

Note that Firefox removed the prompt in bug 469158...

Comment 26

[Stefan [:stefanh]]

We really need to relnote this for 2.1b3. Jens, if you're doing the relnotes (assumption is based upon the fact that you did it the last time), ping me if you need more info.

Comment 27

[Stefan [:stefanh]]

fwiw, atm my only idea on solving this is to make the dialog appear earlier (when "quit-application-granted" happens).

Comment 28

[Justin Wood (:Callek)]

(In reply to comment #27)
> fwiw, atm my only idea on solving this is to make the dialog appear earlier
> (when "quit-application-granted" happens).

If that works lets do it, we need this owned and/or we will be *forced* to ship with it (not something I want to do)

Comment 29

[Stefan [:stefanh]]

(In reply to comment #28)
> (In reply to comment #27)
> > fwiw, atm my only idea on solving this is to make the dialog appear earlier
> > (when "quit-application-granted" happens).
> 
> If that works lets do it, we need this owned and/or we will be *forced* to ship
> with it (not something I want to do)

Neil, what's your opinion on this? Should we instead remove the prompt/dialog (comment #25)? Jftr, I'm unsure if I'll be able to look at this until the beginning of May (leaving for holiday in a couple of days).

Comment 30

[[e-mail]]

If we make the dialog appear earlier, we'll fail to clear history...

Comment 31

[Stefan [:stefanh]]

(In reply to comment #30)
> If we make the dialog appear earlier, we'll fail to clear history...
Ah, right - I should have noticed that.

Comment 32

[[e-mail]]

OK, so I did some debugging, and the last place you can safely spin the event loop (as from a dialog) is from the quit-application notification.

Comment 33

[[e-mail]]

Attachment: Sanitizer rewrite

I had to rewrite the sanitizer so that I could prompt at quit time (while it's still safe, which is what we used to do) but still sanitise on shutdown.

Comment 34

[Stefan [:stefanh]]

Comment on attachment 527511 [details] [diff] [review]
Sanitizer rewrite

JFTR, I won't be able to look at this until I get back from my vacation.

Comment 35

[[e-mail]]

Comment on attachment 527511 [details] [diff] [review]
Sanitizer rewrite

>+    if (Sanitizer.doPendingSanitize())
Oops, doPendingSanitize returns null on success, so this needs an extra !

Comment 36

[Ian Neal]

Comment on attachment 527511 [details] [diff] [review]
Sanitizer rewrite

One strange thing I found, in the dialog the "Download History" is disabled (either via the Tools item or the Clear Now button), yet in the pref pane it is enabled - doesn't look like we check the clearability in the prefpane.
r=me

Comment 37

[[e-mail]]

Pushed changeset 69c0e7fa9ed8 to comm-central.

Pushed changeset ec95fc0f4b7d to releases/comm-2.0

I get the feeling I should set some sort of flag for this but I can't see one.

Comment 38

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #37)
> I get the feeling I should set some sort of flag for this but I can't see one.

You sh/could set the Target Milestone, or what where you thinking about?

BTW, does the "relnote" keyword still apply with the patches in?

Comment 39

[Stefan [:stefanh]]

Michael, if you're able to verify that this is fixed, fewl free to do so - I won't have access to a mac until I'm back from vacation.

Comment 40

[Michal Urban]

@Stefan:

As I don't see any new beta/candidate build yet, I tried the today's nightly. The bug seems to be gone now - with both options checked the dialog appears and browser closes without hanging/crashing.

I'm not sure if you want to move this to VERIFIED based on the nightly, I guess I'll leave that to you.

Comment 41

[Stefan [:stefanh]]

Comment on attachment 527511 [details] [diff] [review]
Sanitizer rewrite

Thanks for testing Michael. 

Post-checkin r+, This works fine on mac now - no hang and everything seems to be cleared (assuming Ian went through the code).

Re comment #36, We shouldn't check the clearability in the prefpane (then you wouldn't be able to set the pref to clear history if you at that moment didn't had any history).

Comment 42

[Stefan [:stefanh]]

Verifying this based on comment #40 and my own testing.