408604 - Crash [@ 0x0] when restarting for updates - metrics.dll

Status: RESOLVED INCOMPLETE

(Toolkit Graveyard :: Data Collection/Metrics, defect)

Description

[(not currently active) Ted Mielczarek]

I've been crashing during the update restart process lately, and I finally looked at one of my Breakpad reports, and most of the top of the stack is in metrics.dll.  Without symbols that's not very useful, but I figured I'd file it anyway.

Comment 1

[Alex Polvi]

luser, thanks for the report. I've heard a little mumbling of other crashes... we'll try to get to the bottom of this.

Comment 2

[Pete Collins (MDG)]

over to Jan for now ...

Comment 3

[Bob Clary [:bc] (inactive)]

I saw this on linux (fedora7) as well.

Comment 4

[Al Billings [:abillings - ex-MoCo]]

I get it on Leopard. I finally had to uninstall the extension.

Comment 5

[Pete Collins (MDG)]

I think this crash is a result of the observer service being gone. I have a path that holds a reference to it I can dig up when I get back ...

Comment 6

[(not currently active) Ted Mielczarek]

I started crashing at startup, it doesn't look like exactly the same stack.  I had to disable the extension so I could use my browser.

Crash report:
http://crash-stats.mozilla.com/report/index/e96a70a6-aff5-11dc-9a6a-001a4bd43e5c

Comment 7

[Alex Polvi]

I'm seeing this too with the latest nightly.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2007122104 Minefield/3.0b3pre

Had to uninstall...

Comment 8

[Pete Collins (MDG)]

I can't reproduce any specific steps to to ensure a consistent crash?

Comment 9

[(not reading, please use [e-mail] instead)]

See http://crash-stats.mozilla.com/report/index/88b7b3b8-b290-11dc-a3a0-001a4bd43ed6 for another example of a libmetrics crash.

Could metrics be calling nsNavHistoryContainerResultNode::GetContainerOpen() with null? 

Comment 10

[Pete Collins (MDG)]

Attachment: crash data

Comment 11

[Pete Collins (MDG)]

Attachment: crash patch 1

this patch seems to fix the crash for me ...

Comment 12

[Pete Collins (MDG)]

Ok, I think I've come to the bottom of this crasher issue. What I think is happening is our binaries are falling out of sync with the nightly updates. I guess enough has changed in the tree to cause our binary libs to crash. 

If you notice the crash wasn't observed until people started updating their Minefield distros. 

Syncing with the latest trunk has put us back into stable territory. So this is an issue that I think will continue to happen. 

Maybe it would be prudent for us to roll out weekly updated XPI builds of metrics so we all stay in sync and everyone can continue to use the extension and provide valuable date, QA, etc. 

Thoughts?

Comment 13

[Jan Varga [:janv]]

Hmm, the extension uses xpcom glue code and links only with NSPR libs. However it uses many unfrozen interfaces that changes very often like nsINavHistoryService.

Comment 14

[Benjamin Smedberg]

Using unfrozen interfaces shouldn't be a problem as long as

1) any time the interface changes the UUID changes (which is a review guideline)
2) If you GetService/CreateInstance, you null-check the result before using it
3) If you get a pointer from another interface, call QI on it before using it:

nsCOMPtr<nsINavHistoryService> navs = do_GetService(navhistoryservice);
if (!navs)
  return NS_ERROR_FAILURE;

nsCOMPtr<nsINavHistoryQuery> navq;
nsresult rv = navs->GetNewQuery(getter_AddRefs(navq));
if (NS_FAILED(rv))
  return rv;

nsCOMPtr<nsINavHistoryQuery> checked_navq = do_QueryInterface(navq);
if (!checked_navq) {
  // nsINavHistoryQuery changed, and failed QI... "navq" is a bad pointer,
  // don't use it
  return NS_ERROR_FAILURE;
}

Comment 15

[Pete Collins (MDG)]

Ok, I did some testing today and without the patch attached to this bug, I am seeing the crash again. 

Comment 16

[Robert Kaiser]

This "metrics" extension stuff is in graveyard, so I guess it's something not relevant today. Resolving the bug to get it off the "open crashes" list, please reopen if it's still relevant.