Fix getStats() freeze bug affecting Chromium but not WebRTC standalone.

PeerConnection::Close() is, per-spec, a blocking operation.
Unfortunately, PeerConnection is implemented to own resources used by
the network thread, and Close() - on the signaling thread - destroys
these resources. As such, tasks run in parallel like getStats() get into
race conditions with Close() unless synchronized. The mechanism in-place
is RTCStatsCollector::WaitForPendingRequest(), it waits until the
network thread is done with the in-parallel stats request.

Prior to this CL, this was implemented by performing
rtc::Thread::ProcessMessages() in a loop until the network thread had
posted a task on the signaling thread to say that it was done which
would then get processed by ProcessMessages(). In WebRTC this works, and
the test is RTCStatsIntegrationTest.GetsStatsWhileClosingPeerConnection.

But because Chromium's thread wrapper does no support
ProcessMessages(), calling getStats() followed by close() in Chrome
resulted in waiting forever (https://crbug.com/850907).

In this CL, the process messages loop is removed. Instead, the shared
resources are guarded by an rtc::Event. WaitForPendingRequest() still
blocks the signaling thread, but only while shared resources are in use
by the network thread. After this CL, calling WaitForPendingRequest() no
longer has any unexpected side-effects since it no longer processes
other messages that might have been posted on the thread.

The resource ownership and threading model of WebRTC deserves to be
revisited, but this fixes a common Chromium crash without redesigning
PeerConnection, in a way that does not cause more blocking than what
the other PeerConnection methods are already doing.

Note: An alternative to using rtc::Event is to use resource locks and
to not perform the stats collection on the network thread if the
request was cancelled before the start of processing, but this has very
little benefit in terms of performance: once the network thread starts
collecting the stats, it would use the lock until collection is
completed, blocking the signaling thread trying to acquire that lock
anyway. This defeats the purpose and is a riskier change, since
cancelling partial collection in this inherently racy edge-case would
have observable differences from the returned stats, which may cause
more regressions.

Bug: chromium:850907
Change-Id: Idceeee0bddc0c9d5518b58a2b263abb2bbf47cff
Reviewed-on: https://webrtc-review.googlesource.com/c/121567
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26707}
4 files changed
tree: 44167e239467e9e739c64d9bdd81b8a234be4e65
  1. api/
  2. audio/
  3. build_overrides/
  4. call/
  5. common_audio/
  6. common_video/
  7. data/
  8. examples/
  9. logging/
  10. media/
  11. modules/
  12. p2p/
  13. pc/
  14. resources/
  15. rtc_base/
  16. rtc_tools/
  17. sdk/
  18. stats/
  19. style-guide/
  20. system_wrappers/
  21. test/
  22. tools_webrtc/
  23. video/
  24. .clang-format
  25. .git-blame-ignore-revs
  26. .gitignore
  27. .gn
  28. .vpython
  29. abseil-in-webrtc.md
  30. AUTHORS
  31. BUILD.gn
  32. CODE_OF_CONDUCT.md
  33. codereview.settings
  34. common_types.h
  35. DEPS
  36. ENG_REVIEW_OWNERS
  37. LICENSE
  38. license_template.txt
  39. native-api.md
  40. OWNERS
  41. PATENTS
  42. PRESUBMIT.py
  43. presubmit_test.py
  44. presubmit_test_mocks.py
  45. pylintrc
  46. README.chromium
  47. README.md
  48. style-guide.md
  49. WATCHLISTS
  50. webrtc.gni
  51. whitespace.txt
README.md

WebRTC is a free, open software project that provides browsers and mobile applications with Real-Time Communications (RTC) capabilities via simple APIs. The WebRTC components have been optimized to best serve this purpose.

Our mission: To enable rich, high-quality RTC applications to be developed for the browser, mobile platforms, and IoT devices, and allow them all to communicate via a common set of protocols.

The WebRTC initiative is a project supported by Google, Mozilla and Opera, amongst others.

Development

See http://www.webrtc.org/native-code/development for instructions on how to get started developing with the native code.

Authoritative list of directories that contain the native API header files.

More info