435122 - Use TraceMalloc to get stack traces of potential deadlocks

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

This patch makes it possible to see the full stack trace for both locks implicated in a potential deadlock warning. You have to --enable-trace-malloc in order for this to all work, and then you have to run the resulting stacks through dbaron's fix-<myOS>-stack.pl script found in mozilla/tools/rb.

Thanks to dbaron and brendan this code was ready to go. It just needed to get hooked up.

Comment 1

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

The nsAutoLock changes look fine to me.  But I wonder why you needed to add FORCE_STATIC_LIB to trace-malloc -- that makes trace-malloc hacking a good bit harder, since it requires rebuilding in a whole bunch of places.

Comment 2

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

Comment on attachment 322025 [details] [diff] [review]
Patch, v1

OK, now I see what you're doing with the linker stuff -- just moving trace-malloc back into xpcom (where it used to be).  That's fine with me, though you should run it by Benjamin.

I'm still curious what's up with the changes to xpcom/tests/Makefile.in and xpcom/proxy/tests/Makefile.in, though.

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Bah, those are totally unnecessary! I'll remove those pronto.

Comment 4

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

To make a clean tree able to compile, you also need to move tools/trace-malloc/lib from tier_gecko in toolkit/toolkit-tiers.mk to tier_xpcom (at some point before xpcom/build) in xpcom/build.mk .  You probably also need to test that.

It might be worth waiting to see if Benjamin's OK with the approach before doing all that, though.

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #4)
> you also need to move tools/trace-malloc/lib from tier_gecko in
> toolkit/toolkit-tiers.mk to tier_xpcom (at some point before xpcom/build)
> in xpcom/build.mk .

Unless I'm misunderstanding you that has already been done in this patch, and it was suggested by Benjamin.

Comment 6

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

Oh, right, never mind.

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

Here's the same patch with the extraneous REQUIRES removed, oops.

Comment 8

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

Comment on attachment 322132 [details] [diff] [review]
Patch, v1

r=dbaron, although I think the REQUIRES in xpcom/build is probably unnecessary as well

Comment 9

[Benjamin Smedberg]

Comment on attachment 322132 [details] [diff] [review]
Patch, v1

Does linking tracemalloc into libxul actually work!? I thought that the symbol redirection only worked correctly when the final binary linked the tracemalloc lib.

Comment 10

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

It certainly used to work fine the way this bug makes it work -- it used to be part of XPCOM until dougt took it out -- see bug 186585.

Comment 11

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #8)
> I think the REQUIRES in xpcom/build is probably unnecessary
> as well

It is, actually. It won't compile otherwise.

Pushed to moz-central.