Closed Bug 848423 Opened 7 years ago Closed 7 years ago

Convert select WebRTC objects to use nsRefPtr<> instead of linked_ptr<>

Categories

(Core :: WebRTC: Signaling, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: abr, Assigned: abr)

Details

(Whiteboard: [webrtc][blocking-webrtc+][qa-])

Attachments

(1 file, 3 obsolete files)

We're running into thread safety issues involving the use of linked_ptr<>
that can be obviated by going to a thread-safe reference-counted-pointer.
The path of least resistance seems to be using the nsRefPtr<> model.
Work in progress; currently only converts CC_CallPtr (since that's where
our pain lies). Not actually threadsafe yet, but easy to make so.
Attachment #721771 - Attachment is obsolete: true
Attachment #721797 - Attachment is obsolete: true
Attachment #721799 - Flags: review?(ethanhugg)
Marking as blocking+ due to intermittent thread hangs
Whiteboard: [webrtc][blocking-webrtc+]
Priority: -- → P2
Comment on attachment 721799 [details] [diff] [review]
Change nearly all WebRTC instances of linked_ptr<> to nsRefPtr<>

Review of attachment 721799 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.  It's good that we're fixing this. Built and tested on Linux64.

::: media/webrtc/signaling/src/softphonewrapper/CC_SIPCCCallServerInfo.h
@@ +10,5 @@
>  #include "common/Wrapper.h"
>  
>  namespace CSF
>  {
> +	DECLARE_NS_PTR(CC_SIPCCCallServerInfo);

This line contains the only tab in this file, if you were so inclined it wouldn't hurt to change it to spaces.  This goes for a few other spots in this patch, but certainly not big deal.
Attachment #721799 - Flags: review?(ethanhugg) → review+
(In reply to Ethan Hugg [:ehugg] from comment #5)

> This line contains the only tab in this file, if you were so inclined it
> wouldn't hurt to change it to spaces.  This goes for a few other spots in
> this patch, but certainly not big deal.

Yeah, I think I'll wait until things are calmer, and then I'll do a big "removing the tabs from SIPCC" patch at some point in the future.
Okay, here's a try with a header file change to hopefully make the Windows builder happy: https://tbpl.mozilla.org/?tree=Try&rev=b4d912bc36d2
Nope, still broken. Well, broken in a different place, so I think I'm on the right path. Here's another try, with a higher chance of success: https://tbpl.mozilla.org/?tree=Try&rev=bb19fb2288d8
Bug 848423 - Change nearly all WebRTC instances of linked_ptr<> to nsRefPtr<>
Attachment #721799 - Attachment is obsolete: true
Comment on attachment 722213 [details] [diff] [review]
Change nearly all WebRTC instances of linked_ptr<> to nsRefPtr<>

Carrying forward r+ from ehugg -- the only difference between this patch and the previous one is the addition of several include files in PeerConnectionCtx.h to satisfy the VC template compiler.
Attachment #722213 - Flags: review+
Depends on: 848966
Re-landed - one of the crashes occurred prior to this sticking on inbound.
https://hg.mozilla.org/integration/mozilla-inbound/rev/74791fb87cf3
https://hg.mozilla.org/mozilla-central/rev/74791fb87cf3
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Whiteboard: [webrtc][blocking-webrtc+] → [webrtc][blocking-webrtc+][qa-]
No longer depends on: 848966
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.