New issue
Advanced search Search tips

Issue 808427 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Data race in blink::ArrayBufferAllocator::AllocateUninitialized

Project Member Reported by ClusterFuzz, Feb 2 2018

Issue description

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race READ 8
Crash Address: 0x7b140001a480
Crash State:
  blink::ArrayBufferAllocator::AllocateUninitialized
  v8::internal::JSTypedArray::MaterializeArrayBuffer
  v8::internal::JSTypedArray::GetBuffer
  
Sanitizer: thread (TSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=533919:533925

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

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, Feb 2 2018

Components: Blink>Bindings Blink>JavaScript
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, Feb 2 2018

Labels: Test-Predator-Auto-Owner
Owner: bbudge@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/eaa730ff5abd585b0b629f07406df6701589e7ff ([ArrayBuffer] Give each SecurityOrigin a Partition for ArrayBuffers.).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
Status: Started (was: Assigned)
Looks like multiple threads accessing the static partitions vector. I wrongly thought defining a thread safe static local made the local thread-safe.
Cc: bbudge@chromium.org
 Issue 808447  has been merged into this issue.
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 5 2018

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

commit cd23fc500688cca826bb6e85ef664c269b1bfcfe
Author: Bill Budge <bbudge@chromium.org>
Date: Mon Feb 05 23:25:15 2018

[ArrayBuffer] Fix a data race when allocating array buffers.

- The vector of partitions may iterated on one thread, while another
  thread is mutating it. Use a mutex to prevent that.

Bug:  chromium:808427 
Change-Id: I4cfee3ad9adeeba6e108f8a79dd1ddef9c418de0
Reviewed-on: https://chromium-review.googlesource.com/899778
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534532}
[modify] https://crrev.com/cd23fc500688cca826bb6e85ef664c269b1bfcfe/third_party/WebKit/Source/platform/wtf/typed_arrays/ArrayBufferContents.cpp

Project Member

Comment 6 by ClusterFuzz, Feb 6 2018

ClusterFuzz has detected this issue as fixed in range 534527:534533.

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race READ 8
Crash Address: 0x7b140001a480
Crash State:
  blink::ArrayBufferAllocator::AllocateUninitialized
  v8::internal::JSTypedArray::MaterializeArrayBuffer
  v8::internal::JSTypedArray::GetBuffer
  
Sanitizer: thread (TSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=533919:533925
Fixed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=534527:534533

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

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 7 by ClusterFuzz, Feb 6 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: Started)
ClusterFuzz testcase 6720385861812224 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 10 2018

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

commit a9b098af6bed01884d88b5863c678b51b3f95f85
Author: Bill Budge <bbudge@chromium.org>
Date: Sat Feb 10 02:16:05 2018

Revert "[ArrayBuffer] Fix a data race when allocating array buffers."

This reverts commit cd23fc500688cca826bb6e85ef664c269b1bfcfe.

Reason for revert: Performance regression:
https://bugs.chromium.org/p/chromium/issues/detail?id=809546

Original change's description:
> [ArrayBuffer] Fix a data race when allocating array buffers.
> 
> - The vector of partitions may iterated on one thread, while another
>   thread is mutating it. Use a mutex to prevent that.
> 
> Bug:  chromium:808427 
> Change-Id: I4cfee3ad9adeeba6e108f8a79dd1ddef9c418de0
> Reviewed-on: https://chromium-review.googlesource.com/899778
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Commit-Queue: Bill Budge <bbudge@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#534532}

TBR=bbudge@chromium.org,haraken@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  chromium:808427 
Change-Id: I5ec463ac45afa88c4bf975cbe353e2a46b1dd8ea
Reviewed-on: https://chromium-review.googlesource.com/912088
Reviewed-by: Bill Budge <bbudge@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535934}
[modify] https://crrev.com/a9b098af6bed01884d88b5863c678b51b3f95f85/third_party/WebKit/Source/platform/wtf/typed_arrays/ArrayBufferContents.cpp

Sign in to add a comment