Last Comment Bug 778791 - GlodaDatastore not shut down properly when opening address book or composer from cmdline
: GlodaDatastore not shut down properly when opening address book or composer f...
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: General (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Thunderbird 17.0
Assigned To: Mike Conley (:mconley) - (needinfo me!)
:
Mentors:
Depends on:
Blocks: 711076
  Show dependency treegraph
 
Reported: 2012-07-30 10:53 PDT by Mike Conley (:mconley) - (needinfo me!)
Modified: 2012-07-30 14:18 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1 (2.58 KB, patch)
2012-07-30 10:56 PDT, Mike Conley (:mconley) - (needinfo me!)
bugmail: review+
Details | Diff | Review
Patch v2 (972 bytes, patch)
2012-07-30 13:26 PDT, Mike Conley (:mconley) - (needinfo me!)
bugmail: review+
Details | Diff | Review

Description Mike Conley (:mconley) - (needinfo me!) 2012-07-30 10:53:59 PDT
STR (using a recent debug build)

1) Open the address book or composer from the cmdline (thunderbird -addressbook or thunderbird -compose)
2) Close the window that spawns

What happens?

Assertion failure: !connections[i]->ConnectionReady(), at /media/Projects/mozilla/thunderbird/mozilla/storage/src/mozStorageService.cpp:853

I've traced this back to GlodaDatastore not being shutdown.

What's expected?

We should should shut down the GlodaDatastore if Gloda is initted from the composer / addressbook standalone windows.
Comment 1 Mike Conley (:mconley) - (needinfo me!) 2012-07-30 10:56:05 PDT
Created attachment 647218 [details] [diff] [review]
Patch v1

This fixes the assertion failure for me in the two cases I mentioned.

I'm just guessing that this is where we'd want to do the shutdown - but if there's a more preferable way of shutting down GlodaDatastore, let me know.
Comment 2 :aceman 2012-07-30 11:44:04 PDT
See also bug 760243.
Comment 3 Andrew Sutherland [:asuth] 2012-07-30 12:45:50 PDT
Comment on attachment 647218 [details] [diff] [review]
Patch v1

indexer.js already listens for quit-application and shuts down the datastore.  I guess you would want to have it listen for profile-before-change as well.  According to https://developer.mozilla.org/en/Observer_Notifications, quit-application is before profile-before-change, so I think it makes sense to still listen for that too.

r=asuth conditional on doing that instead.

Is the command-line handler causing quit-application to never be delivered or has the ordering changed more?  I do know that profile-before-change has been the recent standard for observer notifications for db shutdown, but I forget any nuances...
Comment 4 Mike Conley (:mconley) - (needinfo me!) 2012-07-30 13:26:52 PDT
Created attachment 647275 [details] [diff] [review]
Patch v2

So I dug a little deeper - the reason that the indexer wasn't shutting down GlodaDatastore was because, for the stand-alone address book, the indexer wasn't being loaded / initted.

I traced this to glodaIndexer.js, which imports gloda.js on its own. Changing that to import public.js seemed like the thing to do, and fixes the assertion failure.

Sound ok?
Comment 5 Andrew Sutherland [:asuth] 2012-07-30 13:39:00 PDT
Comment on attachment 647275 [details] [diff] [review]
Patch v2

Yes, this is exactly the right course of action.  Thanks!
Comment 6 Mike Conley (:mconley) - (needinfo me!) 2012-07-30 14:18:45 PDT
This is actually an Activity Manager bug, but since that doesn't have a component on Bugzilla, I'm putting under General.

Anyhow, checked in:

https://hg.mozilla.org/comm-central/rev/3a77dab67b90

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