644637 - new trunk topcrash [@ nsQueryInterfaceWithError::operator()(nsID const&, void**)] called from nsDownload::OnStateChange or [@ nsCOMPtr_base::assign_from_qi_with_error | nsDownload::OnStateChange] or [@ XPCWrappedNative::FlatJSObjectFinalized(JSContext*)]

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

A new trunk #1 topcrash appeared in yesterday afternoon's builds.  The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=200a746e0fac&tochange=4902d72f6072

the crashes are:
https://crash-stats.mozilla.com/report/list?product=Firefox&branch=2.2&query_search=signature&query_type=exact&query=&date=03%2F24%2F2011%2009%3A49%3A50&range_value=30&range_unit=days&hang_type=crash&process_type=browser&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=1&signature=nsQueryInterfaceWithError%3A%3Aoperator%28%29%28nsID%20const%26%2C%20void**%29

it appears to be a null dereference.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

On Mac, it shows up as:

https://crash-stats.mozilla.com/report/list?product=Firefox&branch=2.2&query_search=signature&query_type=exact&query=&date=03%2F24%2F2011%2010%3A00%3A06&range_value=30&range_unit=days&hang_type=crash&process_type=browser&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=1&signature=nsCOMPtr_base%3A%3Aassign_from_qi_with_error%20|%20nsDownload%3A%3AOnStateChange

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is a regression from bug 595785, and I'm a doofus.  Quoting from the context of that patch:

    * The request that's being loaded. Not used after OnStopRequest, so a weak
    * reference suffices. Initialized in OnStartRequest.
    */
   nsIRequest*  mRequest;

That part about "not used after OnStopRequest" is no longer true, of course.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Make sure mRequest lives long enough that we don't hand dead objects around.

Comment 4

[Grigory Mozhaev]

> Created attachment 521537 [details] [diff] [review]
> Make sure mRequest lives long enough that we don't hand dead objects around.

from patch:
> Nulled out in OnStartRequest 

Actually it's initialized in OnStartRequest and nulled out in OnStopRequest (or later)(In reply to comment #3)

Comment 5

[Boris Zbarsky [:bzbarsky]]

Er, yes.  Typo fixed locally.  Thanks for catching that!

Comment 6

[Shawn Wilsher :sdwilsh]

Comment on attachment 521537 [details] [diff] [review]
Make sure mRequest lives long enough that we don't hand dead objects around.

r=sdwilsh

Comment 7

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/projects/cedar/rev/3c5fdab31c67

Comment 8

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/3c5fdab31c67