212767 - Crash when initializing Midas (setting designMode and others)

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Ere Maijala (slow)]

I've experienced a crash with Midas in an intranet application. It works usually
fine, but there is one specific page which causes it to crash. I took the
contents of that page and minimized it to a crashing test case, which I will
attach to this bug. Running trunk build 2003070107 on WinXP, also happens with
20030714.

Comment 1

[Ere Maijala (slow)]

Attachment: Test case

A test case simplified from a real life crash case.

Comment 2

[Matthias Versen [:Matti]]

Attachment: win2k stack

Comment 3

[Matthias Versen [:Matti]]

*** This bug has been marked as a duplicate of 171949 ***

Comment 4

[Ere Maijala (slow)]

Attachment: My stack

My stack is completely different.

Comment 5

[Ere Maijala (slow)]

Reopening. There was yet another place it crashed in talkback 21908653q.

Comment 6

[David Bradley]

Attachment: Purify stack to the error, free, and allocation locations

I'm attaching the purify data, it shows where the object was created and
destroyed that the QI is failing on.

From what I can tell is that an event is fired, and the command controller
still has a reference to the editor which was destroyed when leaving
nsEditingSession::TearDownEditorOnWindow. I suspect maybe there's some kind of
contract between the nsEditingSession and the command controller built, that it
keeps things alive as long as needed. I think the
nsEditingSession::TearDownEditorOnWindow needs to clean up the command
controller before it exits, since I think all references to the object in
question will be gone.

Comment 7

[Kathleen :Brade]

-->me

Comment 8

[Mats Palmgren (inactive)]

Duplicate of bug 211348?

Comment 9

[Ere Maijala (slow)]

Attachment: Patch v1

Need to null out the weak refs to the editor on the controllers before it's
destroyed (nulled out on the docshell). Otherwise there is a small timeframe
when someone can try to use the destroyed editor. This fixes the crash my stack
and the Purify stack show.

Comment 10

[Boris Zbarsky [:bzbarsky]]

It'll take me some time (two weeks or more, possibly) to get to this review --
I'm somewhat behind on reviews at the moment.

For future reference, using the -p flag to diff makes patches like this far more
readable...

Comment 11

[Ere Maijala (slow)]

Attachment: Patch v1 with some more context

Oops, I did it again. This is the same with some more context.

Comment 12

[Kathleen :Brade]

Comment on attachment 140088 [details] [diff] [review]
Patch v1 with some more context

Are there any leaks?  Is "PreDestroy" being called?  Maybe the ordering thing I
was worried about is the selection and doc state listener; I'm not sure
(sorry).

Comment 13

[Ere Maijala (slow)]

No problem, I'll test it.

Comment 14

[Ere Maijala (slow)]

PreDestroy is being called just fine. As far as I can see the patch causes no
problems or changes in behavior.

Comment 15

[Ere Maijala (slow)]

Comment on attachment 140075 [details] [diff] [review]
Patch v1

Seeking r from Brade who seems to know this stuff well :)

Comment 16

[Joe Francis]

looks ok to me.

Comment 17

[Ere Maijala (slow)]

Comment on attachment 140075 [details] [diff] [review]
Patch v1

That would be an r+?

Comment 18

[Kathleen :Brade]

Comment on attachment 140075 [details] [diff] [review]
Patch v1

r=brade (assuming mail compose and composer have both been tested and no
regressions found)

Comment 19

[Ere Maijala (slow)]

Comment on attachment 140075 [details] [diff] [review]
Patch v1

Yes, I wasn't able to find any problems in mail compose or composer. Requesting
approval for 1.7a.

Comment 20

[chris hofmann]

Comment on attachment 140075 [details] [diff] [review]
Patch v1

a=chofmann for 1.7a

Comment 21

[Ere Maijala (slow)]

Fix checked in.

Comment 22

[Bhaskar Nallapeta]

i still see this problem in 1.7.3 on win2k.

Comment 23

[timeless]

so file a new bug with a stack trace.