Closed Bug 625311 Opened 9 years ago Closed 9 years ago

null pointer crash [@ nsThebesDeviceContext::SetDPI]

Categories

(Core :: Graphics, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- ?
status1.9.1 --- ?

People

(Reporter: smaug, Assigned: smaug)

References

Details

Attachments

(1 file)

With the patch for Bug 624549, the following crash happens quite often

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1294906982.1294907354.19293.gz&fulltext=1

0  XUL!nsThebesDeviceContext::SetDPI [nsThebesDeviceContext.cpp:54184cfa6f0e : 650 + 0x3]
    rbx = 0x0000003c   r12 = 0x00001680   r13 = 0x5fbfbde0   r14 = 0x0000003c
    r15 = 0x5fbfbea0   rip = 0x00c61db1   rsp = 0x5fbfbd90   rbp = 0x358a1c30
    Found by: given as instruction pointer in context
 1  XUL!nsThebesDeviceContext::CheckDPIChange [nsThebesDeviceContext.cpp:54184cfa6f0e : 1190 + 0x4]
    rbx = 0x0000003c   r12 = 0x00001680   r13 = 0x03c5f560   r14 = 0x0000003c
    r15 = 0x5fbfbea0   rip = 0x00c61eb2   rsp = 0x5fbfbe20   rbp = 0x358a1c30
    Found by: call frame info
 2  XUL!nsPresContext::PreferenceChanged [nsPresContext.cpp:54184cfa6f0e : 777 + 0x8]
    rbx = 0x5fbfbf50   r12 = 0x5fbfc0c0   r13 = 0x03c5f560   r14 = 0x0000003c
    r15 = 0x5fbfbea0   rip = 0x002030ae   rsp = 0x5fbfbe40   rbp = 0x3b67b800
    Found by: call frame info




Or  0  0x0
    eip = 0x00000000   esp = 0xbf90911c   ebp = 0xbf909198   ebx = 0x034b9288
    esi = 0x0bb4c928   edi = 0x012d6a4e   eax = 0x0fd9ff10   ecx = 0x0000000c
    edx = 0x00000000   efl = 0x00010202
    Found by: given as instruction pointer in context
 1  libxul.so!nsThebesDeviceContext::CheckDPIChange [nsThebesDeviceContext.cpp : 1190 + 0xa]
    eip = 0x023b0ce3   esp = 0xbf9091a0   ebp = 0xbf9091b8
    Found by: previous frame's frame pointer
 2  libxul.so!nsPresContext::PreferenceChanged [nsPresContext.cpp : 777 + 0x17]
    eip = 0x012d66ec   esp = 0xbf9091c0   ebp = 0xbf909268
    Found by: call frame info
 3  libxul.so!nsPresContext::PrefChangedCallback [nsPresContext.cpp : 143 + 0x11]
    eip = 0x012d6ab6   esp = 0xbf909270   ebp = 0xbf909298   ebx = 0x034b9288
    esi = 0x0bb4c928
    Found by: call frame info
Group: core-security
The stack looks strange. There is a null check just before using mWidget.
mWidget handling is suspicious though. It is a raw pointer which is set
in the init but never unset, if I read the code correctly.

Marking ss for now.

Who knows about thebescontext?
That does look suspicious.

We might be able to get rid of the mWidget field, and just pass the widget in where it's needed. nsScreen needs it, so nsScreen could hold a strong reference to the widget.

We can't just make mWidget a strong reference, since that would create a cycle (nsBaseWidget holds a reference to the device context). Maybe we could do that and break the cycle in nsIWidget::Destroy.
I did test strong pointer, and just starting and closing browser didn't leak.
But perhaps I should test something else.
Strange. There should be a simple cycle nsThebesDeviceContext<->nsBaseWidget.
Hmm, neither of those objects have MOZ_COUNT_CTOR, I suspect...
NS_IMPL_ISUPPORTS3 is used and that gives addref/release log.
nsBaseWidget does explicitly NS_IF_RELEASE mContext
...though, if widget::Destroy isn't called, we probably leak.
Blocks: 495920
Attached patch comptrSplinter Review
As far as I see, widget->Destroy is called always.
Either in viewmanager, or in nsTabChild, or in nsWebBrowser::InternalDestroy().
Assignee: nobody → Olli.Pettay
Attachment #504417 - Flags: review?(roc)
Attachment #504417 - Flags: approval2.0?
http://hg.mozilla.org/mozilla-central/rev/acb48b922e91
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
mWidget has been a pointer for a long time. Do we need this fix on the branches as well, or is this a regression from something that isn't going to happen on old versions? Is bug 624569 a cause, or is this just more likely to happen if we change when we run GC?
status1.9.1: --- → ?
status1.9.2: --- → ?
(In reply to comment #11)
> mWidget has been a pointer for a long time.
Since Bug 495920. I don't know what naNativeWidget was-

> or is this just more likely to happen if
> we change when we run GC?
As far as I know, this is why I noticed this.
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.