gohome1984 / google-breakpad

Automatically exported from code.google.com/p/google-breakpad
0 stars 0 forks source link

Random crashes of dump_syms on Mac #337

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I've had several random crashes of dump_syms on Mac OS X 10.6 recently. After 
some debugging it 
looks like it's a premature release of an NSData object in dump_syms.mm.

The method loadModuleInfo loads the dSYM file into memory, and releases it when 
it's done. But 
the entire class never copies any data, but references it via pointers. So by 
the time loadModuleInfo 
finishes and has released its data, it's a mere gamble when one of the 
referencing pointers causes a 
crash.

The attached patch fixes the problem by moving the NSData object into class 
scope, where it lives 
long enough.

Original issue reported on code.google.com by bernd.he...@gmail.com on 7 Oct 2009 at 4:46

Attachments:

GoogleCodeExporter commented 9 years ago
I've noticed this (the bug in the code, not the crashes) from time to time.

The patch looks good as far as it goes, but loadSymbolInfoForArchitecture 
re-maps the
file; this would now be unnecessary, as one still has the original mapping 
available.
Could you address that, as well?

Original comment by jimbla...@gmail.com on 21 Feb 2010 at 6:38

GoogleCodeExporter commented 9 years ago
Dupe of issue 329?

Also jimb has refactored this code immensely, should check if this still exists 
after
all his patches land.

Original comment by ted.mielczarek on 29 Apr 2010 at 6:11

GoogleCodeExporter commented 9 years ago
Probably fixed in r614 or so. Can you test with the latest code? Jim rewrote 
basically the entire OS X dump_syms.

Original comment by ted.mielczarek on 29 Jun 2010 at 4:46

GoogleCodeExporter commented 9 years ago
Thanks to both of you for your work on this. Seeing that code, rewriting seemed 
like the only sane choice, but I didn't have time for that when I ran into the 
issue. I will try to test with the latest version as soon as I find time for it.

But please keep in mind that this came up for me in a production environment, 
not a test case, and was not reproducible in every run. So even if it works for 
me, it's not a proof that it works reliably in every case :-)

Original comment by bernd.he...@gmail.com on 30 Jun 2010 at 8:04

GoogleCodeExporter commented 9 years ago
If we don't have a reproduction method for this, and the code has been 
replaced, I think it makes sense to close this bug; any objections?

Original comment by jimbla...@gmail.com on 30 Jun 2010 at 6:56

GoogleCodeExporter commented 9 years ago
I think it makes perfect sense to close this bug. Even if memory issues show up 
in the rewritten code, it's essentially a new bug without history.

Original comment by bernd.he...@gmail.com on 1 Jul 2010 at 9:05

GoogleCodeExporter commented 9 years ago

Original comment by jimbla...@gmail.com on 1 Jul 2010 at 10:42