410746 - [FIX]NS_IMPL_AGGREGATED produces non-threadsafe addref/release that don't assert

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

I ran into people using loadgroups on non-main threads...  The problem is that loadgroups use NS_IMPL_AGGREGATED, and the Internal::AddRef/Release methods this produces are most definitely not threadsafe, but don't NS_ASSERT_OWNINGTHREAD(), unlike NS_IMPL_ADDREF/RELEASE.

We should either make this refcounting threadsafe, or make it assert.

Comment 1

[Mike Schroepfer]

+ing because this is scary - bz who's a good person to take this?

Comment 2

[Boris Zbarsky [:bzbarsky]]

That's a good question.

Let me try adding the relevant asserts and seeing whether we hit them much.  If we don't, I'll post the patch.  If we do, I'll just try making the refcounting threadsafe.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Doesn't look like we have many consumers for this at all

I do wonder why loadgroup and simple uri use this...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Checked in.