New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 782099 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocked on:
issue 542522
issue webrtc:8571



Sign in to add a comment

CHECK failure: CreateThread failed in platform_thread.cc

Project Member Reported by ClusterFuzz, Nov 7 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6389986323857408

Fuzzer: inferno_layout_test_unmodified
Job Type: windows_asan_content_shell
Platform Id: windows

Crash Type: CHECK failure
Crash Address: 
Crash State:
  CreateThread failed in platform_thread.cc
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=507959:507978

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6389986323857408

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: msrchandra@chromium.org pnangunoori@chromium.org
Components: Blink>WebRTC
Labels: Test-Predator-Wrong CF-NeedsTriage M-63
Unable to provide possible suspect using Predator, CL and Code Search.
Could someone please look into the issue.
Thank You.
Owner: hta@chromium.org
Status: Assigned (was: Untriaged)
I can't find a direct suspect within the range either. Below CL makes the most changes. hta@ can you PTAL?
https://chromium.googlesource.com/chromium/src/+/68e8a89834946651c5ab54de10710be2cc21b68c

Comment 3 by hta@chromium.org, Nov 23 2017

Looking at the testcase: This reproducer basically calls "new webkitRTCPeerConnection" 1000 times, and there's a failure to start a thread.

The failure is logical, since the new code involves more WebRTC-level machinery (and therefore likely more thread-starting) than the previous code, which used a Blink-layer mock.

It ought to happen without ASAN too, but ASAN might slow it down enough that situation gets worse. I'll look at testing what happens at various numbers of creations - might end up proposing to limit the range of this particular test.


Comment 4 by hta@chromium.org, Nov 23 2017

The crash happens reliably on a debug build after creating 1487 PeerConnections. Will look at ways to have creation fail rather than crash.

Comment 5 by hta@chromium.org, Nov 23 2017

Opened spec issue on what error to return - https://github.com/w3c/webrtc-pc/issues/1670

Comment 6 by hta@webrtc.org, Nov 24 2017

Blockedon: webrtc:8571

Comment 7 by hta@chromium.org, Nov 24 2017

Blockedon: 542522
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4ec3abc68a3268c5d24bfd38fca733ac33b3ab8b

commit 4ec3abc68a3268c5d24bfd38fca733ac33b3ab8b
Author: Harald Alvestrand <hta@chromium.org>
Date: Mon Nov 27 17:42:37 2017

Add a test for creating many RTCPeerConnections

This test will verify that we can create a consistent number of
peerconnections on all platforms. If we create more, we may
or may not exceed the number of threads allowed and crash; this
is not tested for.
The intent of this test is to catch regressions that decrease the
maximum number of connections.

Bug:  782099 
Change-Id: I87f776b791509075a52e98b9d0b087186d531993
Reviewed-on: https://chromium-review.googlesource.com/789845
Reviewed-by: Tommi <tommi@chromium.org>
Commit-Queue: Harald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519330}
[add] https://crrev.com/4ec3abc68a3268c5d24bfd38fca733ac33b3ab8b/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-many.html

Comment 9 by hta@chromium.org, Nov 27 2017

We seem to have found the reason it crashes, but it's like an OOM - crash is expected under these circumstances.
I couldn't find the clusterfuzz definitions - is it possible to tell the fuzzer to not try to create more than 500 objects?

Comment 10 by hta@chromium.org, Dec 4 2017

Labels: -Pri-1 -Clusterfuzz -CF-NeedsTriage ClusterFuzz-Ignore Pri-2
The base cause is that creating a PeerConnection starts multiple threads for each PeerConnection.

It seems that we can't change the fuzzer that generates this test, so we're marking this "Clusterfuzz-ignore" and lowering the priority.

We'll revisit once the "blockedon" bugs are resolved.

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cd5a4dfc8194b2fd14922865d346f2db0aa21905

commit cd5a4dfc8194b2fd14922865d346f2db0aa21905
Author: Harald Alvestrand <hta@chromium.org>
Date: Tue Dec 05 13:06:34 2017

Verify garbage collection of RTCPeerConnection objects

Closed objects with no references are garbage collected.
Non-closed objects are not garbage collected.

NOTE: This uses a very primitive mechanism (static variable) for
collecting the count. It should use a more reasonable one, and the
accessor should be marked "only visible in testing". Advice on how
to best do that appreciated.

Bug:  782099 
Change-Id: If44f9e6ecaabc8f0dfebe3ee959c5179308f2b0b
Reviewed-on: https://chromium-review.googlesource.com/806195
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Harald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521679}
[add] https://crrev.com/cd5a4dfc8194b2fd14922865d346f2db0aa21905/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-garbagecollected.html
[modify] https://crrev.com/cd5a4dfc8194b2fd14922865d346f2db0aa21905/third_party/WebKit/Source/modules/BUILD.gn
[modify] https://crrev.com/cd5a4dfc8194b2fd14922865d346f2db0aa21905/third_party/WebKit/Source/modules/modules_idl_files.gni
[modify] https://crrev.com/cd5a4dfc8194b2fd14922865d346f2db0aa21905/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
[modify] https://crrev.com/cd5a4dfc8194b2fd14922865d346f2db0aa21905/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h
[add] https://crrev.com/cd5a4dfc8194b2fd14922865d346f2db0aa21905/third_party/WebKit/Source/modules/peerconnection/testing/InternalsRTCPeerConnection.cpp
[add] https://crrev.com/cd5a4dfc8194b2fd14922865d346f2db0aa21905/third_party/WebKit/Source/modules/peerconnection/testing/InternalsRTCPeerConnection.h
[add] https://crrev.com/cd5a4dfc8194b2fd14922865d346f2db0aa21905/third_party/WebKit/Source/modules/peerconnection/testing/InternalsRTCPeerConnection.idl

Comment 12 by tommi@chromium.org, Dec 18 2017

Cc: tommi@chromium.org
hta and I discussed this and agreed that as a temporary mitigation, until WebRTC shares threads with Chromium via TaskQueue/TaskScheduler, we should limit the number of PeerConnection instances that a render process can create.

So, as the last action for this particular bug, I propose that we implement said limit (at say 500, 750 or whatever hta determines to be reasonable).

Comment 13 by hta@chromium.org, Dec 22 2017

Labels: -ClusterFuzz-Ignore
Status: Fixed (was: Assigned)
Number of PeerConnnections limited to 500.

https://chromium-review.googlesource.com/c/chromium/src/+/838380

Sign in to add a comment