Last Comment Bug 636210 - Hang when quitting SeaMonkey if "Always clear my private data when I close SeaMonkey" is combined with "Ask me before clearing private data"
: Hang when quitting SeaMonkey if "Always clear my private data when I close Se...
Status: VERIFIED FIXED
: hang, regression
Product: SeaMonkey
Classification: Client Software
Component: General (show other bugs)
: Trunk
: x86 All
: -- critical (vote)
: seamonkey2.1final
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
Depends on: 711937
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-23 11:17 PST by Michal Urban
Modified: 2012-01-03 12:01 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
final+


Attachments
SeaMonkey hangs when clearing private data (110.30 KB, image/png)
2011-02-24 07:10 PST, Michal Urban
no flags Details
Info from Apple crash reporter (229.21 KB, text/plain)
2011-02-24 07:11 PST, Michal Urban
no flags Details
Stack from hang (14.54 KB, text/plain)
2011-03-06 04:59 PST, Stefan [:stefanh]
no flags Details
Sanitizer rewrite (13.37 KB, patch)
2011-04-21 05:00 PDT, neil@parkwaycc.co.uk
iann_bugzilla: review+
stefanh: review+
Details | Diff | Splinter Review

Description Michal Urban 2011-02-23 11:17:02 PST
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 2011-02-23 21:51:02 PST
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 2011-02-24 07:09:35 PST
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 2011-02-24 07:10:34 PST
Created attachment 514787 [details]
SeaMonkey hangs when clearing private data
Comment 4 Michal Urban 2011-02-24 07:11:40 PST
Created attachment 514788 [details]
Info from Apple crash reporter
Comment 5 Philip Chee 2011-02-24 20:31:09 PST
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 2011-02-25 08:10:30 PST
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 2011-02-25 08:23:16 PST
I get the same behaviour with all Extensions disabled.
Comment 8 Philip Chee 2011-02-25 20:17:49 PST
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 2011-02-26 10:08:53 PST
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 2011-02-27 09:39:58 PST
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 2011-02-28 06:59:27 PST
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 2011-02-28 08:44:24 PST
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 2011-02-28 10:14:56 PST
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 2011-03-01 10:59:27 PST
Stefan has a Mac, I'll CC him on this bug.
Comment 15 Stefan [:stefanh] 2011-03-04 15:37:10 PST
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 2011-03-04 17:30:05 PST
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] 2011-03-05 04:44:11 PST
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 2011-03-05 08:48:49 PST
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] 2011-03-06 03:23:42 PST
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] 2011-03-06 04:23:46 PST
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] 2011-03-06 04:35:33 PST
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] 2011-03-06 04:59:13 PST
Created attachment 517246 [details]
Stack from hang

Here's a stack from the hang
Comment 23 Michal Urban 2011-03-07 07:48:16 PST
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 neil@parkwaycc.co.uk 2011-03-09 02:18:12 PST
(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 neil@parkwaycc.co.uk 2011-03-09 02:23:39 PST
Note that Firefox removed the prompt in bug 469158...
Comment 26 Stefan [:stefanh] 2011-03-31 14:56:34 PDT
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] 2011-03-31 14:58:55 PDT
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) (Away until Aug 29) 2011-04-18 00:03:44 PDT
(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] 2011-04-18 09:07:09 PDT
(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 neil@parkwaycc.co.uk 2011-04-18 09:08:52 PDT
If we make the dialog appear earlier, we'll fail to clear history...
Comment 31 Stefan [:stefanh] 2011-04-18 09:13:32 PDT
(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 neil@parkwaycc.co.uk 2011-04-20 08:54:30 PDT
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 neil@parkwaycc.co.uk 2011-04-21 05:00:57 PDT
Created attachment 527511 [details] [diff] [review]
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] 2011-04-21 05:10:55 PDT
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 neil@parkwaycc.co.uk 2011-04-22 05:17:14 PDT
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 2011-04-23 12:14:33 PDT
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 neil@parkwaycc.co.uk 2011-04-26 12:54:54 PDT
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) 2011-04-26 13:09:08 PDT
(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] 2011-04-28 15:19:19 PDT
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 2011-05-02 07:53:21 PDT
@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] 2011-05-07 07:46:37 PDT
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] 2011-05-07 07:47:23 PDT
Verifying this based on comment #40 and my own testing.

Note You need to log in before you can comment on or make changes to this bug.