Skip to content

Commit

Permalink
JingleThreadWrapper::IsQuitting() returns false.
Browse files Browse the repository at this point in the history
In https://webrtc-review.googlesource.com/c/src/+/124701, PostTask() is
added to WebRTC threads in order to make it easier to post tasks
asynchronously in third_party/webrtc, and to start discouraging blocking
thread invokes just like they are in Chrome.

Part of that CL adds a DCHECK that the thread is not quitting when
posting in order to avoid race conditions and posting when the thread is
already quit in WebRTC. The JingleThreadWrapper, which implements
rtc::Thread, does not support quitting. Because of a NOTREACHED() it is
not possible to use rtc::Thread::PostTask() in Chrome because of this
DCHECK.

This CL replaces the NOTREACHED() with NOTIMPLEMENTED_LOG_ONCE() in
IsQuitting(). If quitting is not supported it is reasonable to always
return false. (The alternative would be not to DCHECK !IsQuitting() in
PostTask(), but then we'll be warned about less races on platforms that
do support quitting.)

Bug: webrtc:10294
Change-Id: Ibe35c42f6a860bc7cc933c06deeacff5b2be3215
Reviewed-on: https://chromium-review.googlesource.com/c/1491620
Commit-Queue: Henrik Boström <[email protected]>
Reviewed-by: Sergey Ulanov <[email protected]>
Cr-Commit-Position: refs/heads/master@{#636388}
  • Loading branch information
henbos authored and Commit Bot committed Feb 28, 2019
1 parent 39813ef commit cc8c6bc
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 7 deletions.
10 changes: 5 additions & 5 deletions jingle/glue/thread_wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -280,17 +280,17 @@ void JingleThreadWrapper::RunTask(int task_id) {
}
}

bool JingleThreadWrapper::IsQuitting() {
NOTIMPLEMENTED_LOG_ONCE();
return false;
}

// All methods below are marked as not reached. See comments in the
// header for more details.
void JingleThreadWrapper::Quit() {
NOTREACHED();
}

bool JingleThreadWrapper::IsQuitting() {
NOTREACHED();
return false;
}

void JingleThreadWrapper::Restart() {
NOTREACHED();
}
Expand Down
9 changes: 7 additions & 2 deletions jingle/glue/thread_wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,17 @@ class JingleThreadWrapper
uint32_t id,
rtc::MessageData* data) override;

// Following methods are not supported.They are overriden just to
// Quitting is not supported (see below); this method performs
// NOTIMPLEMENTED_LOG_ONCE() and returns false.
// TODO(https://crbug.com/webrtc/10364): When rtc::MessageQueue::Post()
// returns a bool, !IsQuitting() will not be needed to infer success and we
// may implement this as NOTREACHED() like the rest of the methods.
bool IsQuitting() override;
// Following methods are not supported. They are overriden just to
// ensure that they are not called (each of them contain NOTREACHED
// in the body). Some of this methods can be implemented if it
// becomes neccessary to use libjingle code that calls them.
void Quit() override;
bool IsQuitting() override;
void Restart() override;
bool Get(rtc::Message* message, int delay_ms, bool process_io) override;
bool Peek(rtc::Message* message, int delay_ms) override;
Expand Down

0 comments on commit cc8c6bc

Please sign in to comment.