Closed
Bug 68827
Opened 24 years ago
Closed 24 years ago
Crash in XPCOM migrating profiles
Categories
(Core Graveyard :: Profile: BackEnd, defect)
Core Graveyard
Profile: BackEnd
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.1
People
(Reporter: mkaply, Assigned: ccarlen)
Details
(Keywords: crash)
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
Using current nightly on Windows (also recreated on OS/2, so believe to be XP)
Delete moz*.dat and and the MOZILLA directory in the WINDOWS directory.
Start Mozilla.
When the profile migration screen appears, click Manage Profiles.
Then click Exit on the Profile Manager.
Start Mozilla again.
On the migration screen, click Convert Profile.
Trap in XPCOM.
It appears to be caused by having some information about the profile in the
registry but not all of it.
Assignee | ||
Comment 1•24 years ago
|
||
> Delete moz*.dat and and the MOZILLA directory in the WINDOWS directory.
Can you clarify?
For "MOZILLA directory in the WINDOWS directory" do you mean (if running NT4)
Winnt\Profiles\you\Application Data\Mozilla ?
Reporter | ||
Comment 2•24 years ago
|
||
Yes, that's the directory on NT.
Basically you need to remove any remnants that Mozilla had been started before.
Mozilla directory and *.dat
This crash happens with a "clean" start of Mozilla.
The problem is that during the first start of mozilla,
registry entries will be made saying that there
is a 4x profile to migrate. This entry remains even if the
user presses Exit on the Manage Profiles dialog. During
the second start of mozilla, it will call
nsProfileAccess::Get4xProfileInfo as per usual. This will
enumerate the subtrees in the registry.
For each key it will get the profile name. If the call the
ProfileExists() returns non-zero, it will
skip on to process the next key. The code that it is
skipping is where profileItem's are created and
appended to the m4xProfiles list. The migration code
enters into
nsProfile::AutoMigrate() who calls
nsProfile::MigrateAllProfiles(). It expects m4xProfiles to
have at least one item. In the failing case, this isn't
correct.
So, is the fix to remove the profile registry entries if
"Exit" is ever encountered? Would it be
better to change MigrateAllProfiles() so that if there
aren't any m4xProfiles to create them and
THEN process them? I figure this is the least potentially
problematic solution since I don't know
who expects these entries to be there in other situations.
But removing registry entries when
"Exit" button is pushed would be more intuitive. Or maybe
wait and not even put in the registry
entries until after the migration is done.
Any suggestions? I'll do the fix if you want, but I need
to know what the best fix would be. I'm not that familiar
with other considerations that might need to be taken into
account (like what if the user does some profile renaming
and deletion prior to hitting Exit, do we honor that,
etc.).
Also contributing to this problem is that inside
nsProfileAccess::FillProfileInfo(), mNumOldProfiles can be
incremented (as it is in this case) WITHOUT an item being
added to the m4xProfiles list. If this is the counter the
represents the list, this seems like a pretty risky thing
to be doing. Seems like maybe m4xCount would be the better
counter to use in the for loop in MigrateAllProfiles().
But in this case that would mean that no profiles would be
migrated since m4xCount is zero.
Assignee | ||
Comment 5•24 years ago
|
||
I see the problem: Basically the nsProfileAccess class (despite some earlier
cleanup) is a mess. There are a number of instance variables in it which are
used to keep count of the items in lists. There is both m4xCount and
mNumOldProfiles, and worse, mNumOldProfiles is public - sheesh. Looking at
nsProfileAccess::GetNum4xProfiles(), it does the
right thing by enumerating the list and looking at whether or not the profile
element has been migrated or not. When profile info is migrated by
nsProfileAccess::MigrateProfileInfo(), it is immediately stuffed into the
current list by UpdateProfileArray(). Once that happens, m4xCount and
nNumOldProfiles should be irrelevant. The fact that
nsProfile::MigrateAllProfiles() is looking directly at instance variables in
nsProfileAccess which are themselves questionable is wrong. I've gotta clean
this up once and for all.
Assignee | ||
Comment 6•24 years ago
|
||
Assignee | ||
Comment 7•24 years ago
|
||
The patch above fixes this by not looking at instance variables which it
shouldn't be. Sent to reporter and tested.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9
Assignee | ||
Comment 9•24 years ago
|
||
Assignee | ||
Comment 10•24 years ago
|
||
The 2nd patch fixes the problem but also cleans up nsProfileAccess. There were a
number of instance variables which were used to keep track of the items in an
array. The patch just uses Count() and avoids synchronization problems. Patch
also removes a redundant list and makes nearly all member vars and meny methods
private.
Assignee | ||
Comment 11•24 years ago
|
||
CC'ing Bhuvan and Seth for review/sr.
Updated•24 years ago
|
Assignee | ||
Comment 12•24 years ago
|
||
Bhuvan, Seth - any time to review?
Comment 13•24 years ago
|
||
r=valeski.
Comment 14•24 years ago
|
||
racham knows this code best. waiting for a review from him.
Comment 15•24 years ago
|
||
Conrad,
Changes look good. As you are cleaning up the count variables, it will be best
if you can run some regular profilemanager basic tests (you may have already
done). Tests like migrate 1/more profiles, create profile & quit & launch,
delete profiles.
r=racham.
Comment 16•24 years ago
|
||
sr=sspitzer
but please do a lot of testing.
racham, is http://www.mozilla.org/profilemanager/checklist.txt still the correct
checklist?
Comment 17•24 years ago
|
||
* One can delete unmigrated profile from the list now. It just takes it out of
the registry.dat file.
* -CreateProfile, creates profile and quits the app.
Those are the 2 things I noticed being changed since that document is posted. I
have requested for an account on gila server a while ago and it never
materialized. I will follow up on that.
Comment 18•24 years ago
|
||
racham: send me an email and I'll fix that documentation for you tomorrow.
Assignee | ||
Comment 19•24 years ago
|
||
Seth, I have a gila account and should probably do that. In addition to the
updates Bhuvan mentioned, there's another change: The document mentions
mozregistry. It's now registry.dat. The file mozregistry will never be looked at
by profile mgr.
Comment 20•24 years ago
|
||
ccarlen, you're a much better person to fix up the checklist since you're
hacking in that code. thanks for taking care of it.
after updating checklist.txt and running through them, please update this bug
with the results. a successful run of those tests will put all our minds at
ease.
Assignee | ||
Comment 21•24 years ago
|
||
OK - ran it through the checklist. No problems were found with the code being
checked in for this bug. Two others were:
(1) from my checkin for bug 60182 - deleting a profile and choosing *not* to
delete the files causes nothing to happen - an uninitialized rv was being
returned when in fact everything was NS_OK. I'll sneak that fix in with this one
since it's a one-liner.
(2) When renaming a profile, the name does not update in the profile mgr UI even
though the profile is in fact renamed and NS_OK is returned to frontend. A build
of NS6 from a month ago has the same problem. I'll file a new bug on that one.
(3) This bit from the checklist:
2.
mozilla
case 1 : CreateProfileWizard is launched.
I don't think is accurate. If launched with no args, the profile wizard does not
come up. There are several possibilities that can happen, depending on the
existence of 4x profiles, current profiles, etc. But AFAIK, launching mozilla
with no args should not show the CreateProfileWizard.
Assignee | ||
Comment 22•24 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Whiteboard: awaiting reviews
Comment 23•24 years ago
|
||
Conrad, you are right. CreateProfile Wizard will not come up when launched with
mozilla with no args. Infact, the case in the question there was with no
profiles (in the past we launched the wizard when no 6.0 profiles are
found...that is really distant past..we need to definitely change the doc now).
Seth at later point implemented force migration which when launched with no 6.0
profiles forces migration (all those who delete their registry and don't know
how to remigrate are taken care of). If no 4.x profile(s) then, we silently
create a default 6.0 profile and launch the browser. you know all these details...
Anyway, I had a skeleton document covering various sections of the
ProfileManager. I will pass it on to you. You can add to/modify the document
with latest and greatest details and post it on gila server. Thanks for the help.
Updated•9 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•