Bug 187402 – Make RealtimeOutgoingVideoSource use DestructionThread::Main

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

Make RealtimeOutgoingVideoSource use DestructionThread::Main

[youenn fablet]

Created attachment 344442 [details]
Patch

[Chris Dumez]

Comment on attachment 344442 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=344442&action=review

> Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:57
> +        auto result = hasOneRef() ? rtc::RefCountReleaseStatus::kDroppedLastRef : rtc::RefCountReleaseStatus::kOtherRefsRemained;

Is this thread safe? If Release() can be called from various threads, then a ref() / deref() can happen on another thread after your hasOneRef() check.

[youenn fablet]

> View in context:
> https://bugs.webkit.org/attachment.cgi?id=344442&action=review
> 
> > Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:57
> > +        auto result = hasOneRef() ? rtc::RefCountReleaseStatus::kDroppedLastRef : rtc::RefCountReleaseStatus::kOtherRefsRemained;
> 
> Is this thread safe? If Release() can be called from various threads, then a
> ref() / deref() can happen on another thread after your hasOneRef() check.

Well, this whole API is not really thread safe but we need to return something as an API contract. Even if we return the true value at the time we did defer(), it will be useless for the caller, except maybe for logging.
 
I am fine continuing returning rtc::RefCountReleaseStatus::kOtherRefsRemained like done before.

[WebKit Commit Bot]

Comment on attachment 344442 [details]
Patch

Clearing flags on attachment: 344442

Committed r233602: <https://trac.webkit.org/changeset/233602>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/41914556>