If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[FIXr]Crash after changing "minimum font size" and pressing OK [@ operator | nsQueryInterface::operator]

VERIFIED FIXED in mozilla1.3alpha

Status

()

Core
Layout
P1
critical
VERIFIED FIXED
16 years ago
14 years ago

People

(Reporter: alge, Assigned: bz)

Tracking

({crash, topcrash})

Trunk
mozilla1.3alpha
x86
All
crash, topcrash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

16 years ago
On linux/2002061014.
Not easily reproduceable (I tried for 10 minutes before giving up)

talkback: TB7211829H
(Reporter)

Comment 1

16 years ago
CCing asa for TB:  TB7211829H

Updated

16 years ago
Keywords: crash
operator []()
nsCOMPtr_base::assign_from_helper()
nsPresContext::PreferenceChanged()
nsPresContext::PrefChangedCallback()
pref_DoCallback()
pref_HashPref()
PREF_SetIntPref()
nsPrefBranch::SetIntPref()
nsPrefService::SetIntPref()
nsPref::SetIntPref()
XPTC_InvokeByIndex()
XPCWrappedNative::CallMethod()
XPC_WN_CallMethod()
js_Invoke()
js_Interpret()
js_Invoke()
js_InternalInvoke()
JS_CallFunctionValue()
nsJSContext::CallEventHandler()
GlobalWindowImpl::RunTimeout()
GlobalWindowImpl::TimerCallback()
nsTimerImpl::Fire()
handleTimerEvent()
PL_HandleEvent()
PL_ProcessPendingEvents()
nsEventQueueImpl::ProcessPendingEvents()
event_processor_callback()
our_gdk_io_invoke()
libglib-1.2.so.0 + 0xee00 (0x4037de00)
libglib-1.2.so.0 + 0x104c8 (0x4037f4c8)
libglib-1.2.so.0 + 0x10ad3 (0x4037fad3)
libglib-1.2.so.0 + 0x10c6c (0x4037fc6c)
libgtk-1.2.so.0 + 0x8d7f7 (0x402a07f7)
nsAppShell::Run()
nsAppShellService::Run()
main1()
main()
libc.so.6 + 0x1914f (0x4049914f)

Comment 3

16 years ago
layout
Assignee: ben → attinasi
Component: Preferences → Layout
QA Contact: sairuh → petersen
Summary: Crash after changing "minimum font size" and pressing OK → Crash after changing "minimum font size" and pressing OK [@operator []()]

Comment 4

15 years ago
I can't reproduce in the OS X 2002-07-12-08 Build. Is this still occur in the
latest linux build ?
Status: UNCONFIRMED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 5

15 years ago
I could never reproduce.. that doesn't mean there's not a bug, though (:
Reopening. This crash just occured for me with 2002102508 on Win2k. Talkback ID
is TB13115185M.

(dupe of bug 144479?)

OS -> All

CC: Boris, as he applied the fix to (related?) bug 144479.
Status: RESOLVED → UNCONFIRMED
OS: Linux → All
Resolution: WORKSFORME → ---
Confirming.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I've been seeing this on and off as well... stephend, could you get the stack,
possibly?
Keywords: stackwanted
Whiteboard: TB13115185M
0x0207b7fc
nsPresContext::PreferenceChanged
[d:/builds/seamonkey/mozilla/layout/base/src/nsPresContext.cpp, line 607]
nsPresContext::PrefChangedCallback
[d:/builds/seamonkey/mozilla/layout/base/src/nsPresContext.cpp, line 106]
pref_DoCallback [d:/builds/seamonkey/mozilla/modules/libpref/src/prefapi.cpp,
line 1188]
pref_HashPref [d:/builds/seamonkey/mozilla/modules/libpref/src/prefapi.cpp, line
1074]
PREF_SetIntPref [d:/builds/seamonkey/mozilla/modules/libpref/src/prefapi.cpp,
line 561]
nsPrefBranch::SetIntPref
[d:/builds/seamonkey/mozilla/modules/libpref/src/nsPrefBranch.cpp, line 259]
nsPrefService::SetIntPref
[d:/builds/seamonkey/mozilla/modules/libpref/src/nsPrefService.h, line 57]
nsPref::SetIntPref [d:/builds/seamonkey/mozilla/modules/libpref/src/nsPref.cpp,
line 245]
XPTC_InvokeByIndex
[d:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 106]
XPCWrappedNative::CallMethod
[d:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2014]
XPC_WN_CallMethod
[d:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1282]
js_Invoke [d:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 841]
js_Interpret [d:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 2804]
js_Invoke [d:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 857]
js_InternalInvoke [d:/builds/seamonkey/mozilla/js/src/jsinterp.c, line 932]
JS_CallFunctionValue [d:/builds/seamonkey/mozilla/js/src/jsapi.c, line 3433]
nsJSContext::CallEventHandler
[d:/builds/seamonkey/mozilla/dom/src/base/nsJSEnvironment.cpp, line 1044]
GlobalWindowImpl::RunTimeout
[d:/builds/seamonkey/mozilla/dom/src/base/nsGlobalWindow.cpp, line 4634]
GlobalWindowImpl::TimerCallback
[d:/builds/seamonkey/mozilla/dom/src/base/nsGlobalWindow.cpp, line 4981]
nsTimerImpl::Fire [d:/builds/seamonkey/mozilla/xpcom/threads/nsTimerImpl.cpp,
line 368]
nsAppShell::Exit [d:/builds/seamonkey/mozilla/widget/src/windows/nsAppShell.cpp,
line 288]
nsAppShellService::Run
[d:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp, line 472]
main1 [d:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1538]
main [d:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1886]
WinMain [d:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1906]
WinMainCRTStartup()
KERNEL32.DLL + 0x1ca90 (0x77e9ca90) 
Keywords: stackwanted
Whiteboard: TB13115185M
ccing jst because I think he's best qualified to tell me whether I'm on crack...
Created attachment 104348 [details] [diff] [review]
I _think_ this should fix it

I can't reproduce this now... fun... In any case, I'm thinking that the problem
here is that the mContainer goes away (gets destroyed); probably this is the
prefs dialog closing.  This calls Destroy() on the doc viewer, which calls
Destroy() on the presshell and then sets the mContainer on the prescontext to
null.

I suspect that this races with the prefchange notifications (not sure how;
maybe the event queue stuff in nsPresShell::Destroy()?) and we get the
prefchange notification when mContainer is still non-null but no longer points
to anything useful.  Which explains the weird stacks with crashes in
nsCOMPtr_base::assign_from_helper and such.

If that is the case, changing to nsWeakPtr should help the situation (not to
mention just being safer no matter what).  The other alternative would be to
bail out if mPresShell is null, of course.
Blocks: 156486
-> Alex
Assignee: attinasi → alexsavulov
Priority: -- → P2
Target Milestone: --- → mozilla1.3alpha
nsbeta1+
Keywords: nsbeta1+

Comment 14

15 years ago
Comment on attachment 104348 [details] [diff] [review]
I _think_ this should fix it

r= alexsavulov

Boris, this looks very reasonable to me. I wasn't able to repro the crash and
this seems to be the only acceptable thing to do since we (or at least I) don't
know what is the reason for the race situation. and as you say is more safer
anyway. i'll say let's put the fix in for this bug report close both bugs with
a note to reopen one of them if the crash is still there. 

(hmmm, this "Comment on the bug" field is still too wide! :-) )
Attachment #104348 - Flags: review+
taking, since it's my patch...

The one issue that's possible here is a performance one.... if so, the only
place it would really matter is in font style resolution.  I'm pretty sure it
should not be a problem, but I've been unable to come up with a good way to
measure it.
Assignee: alexsavulov → bzbarsky
Priority: P2 → P1
Summary: Crash after changing "minimum font size" and pressing OK [@operator []()] → [FIX]Crash after changing "minimum font size" and pressing OK [@operator []()]
Comment on attachment 104348 [details] [diff] [review]
I _think_ this should fix it

> NS_IMETHODIMP
> nsPresContext::GetContainer(nsISupports** aResult)
> {
>   NS_PRECONDITION(aResult, "null out param");
>+  nsCOMPtr<nsISupports> container = do_QueryReferent(mContainer);
>+  *aResult = container;
>+  NS_IF_ADDREF(*aResult);
>   return NS_OK;
> }

How about

NS_IMETHODIMP
nsPresContext::GetContainer(nsISupports** aResult)
{
  NS_PRECONDITION(aResult, "null out param");
  if (!mContainer) {
    *aResult = nsnull;
    return NS_OK;
  }
  return CallQueryReferent(mContainer, aResult);
}

Other than that, sr=dbaron.  (The current form is OK too, but I'm not a big fan
of the extra refcounting.)
Attachment #104348 - Flags: superreview+
Created attachment 104713 [details] [diff] [review]
good point.  This uses CallQueryReferent

It seems to need the .get() to compile, though....
Attachment #104348 - Attachment is obsolete: true
Comment on attachment 104713 [details] [diff] [review]
good point.  This uses CallQueryReferent

marking the reviews
Attachment #104713 - Flags: superreview+
Attachment #104713 - Flags: review+
Summary: [FIX]Crash after changing "minimum font size" and pressing OK [@operator []()] → [FIXr]Crash after changing "minimum font size" and pressing OK [@operator []()]
checked in for 1.3a
Status: NEW → RESOLVED
Last Resolved: 15 years ago15 years ago
Resolution: --- → FIXED

Comment 20

15 years ago
*** Bug 156486 has been marked as a duplicate of this bug. ***

Comment 21

15 years ago
Glad this fix is coming soon. The crash still happens in Release 1.2.1

Comment 22

15 years ago
*** Bug 185444 has been marked as a duplicate of this bug. ***

Comment 23

15 years ago
Adding topcrash keyword and nsQueryInterface::operator to summary for future
reference.  This crash *was* a topcrash on the MozillaTrunk.  Haven't seen any
crashes in a while though...so v.fixed.
Status: RESOLVED → VERIFIED
Keywords: topcrash
Summary: [FIXr]Crash after changing "minimum font size" and pressing OK [@operator []()] → [FIXr]Crash after changing "minimum font size" and pressing OK [@ operator | nsQueryInterface::operator]

Comment 24

14 years ago
*** Bug 100784 has been marked as a duplicate of this bug. ***
Crash Signature: [@ operator | nsQueryInterface::operator]
You need to log in before you can comment on or make changes to this bug.