New issue
Advanced search Search tips

Issue 788495 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Dec 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Mac
Pri: 1
Type: Bug



Sign in to add a comment

CHECK failure: !CallbackFunction().IsEmpty() in v8_mutation_callback.cc

Project Member Reported by ClusterFuzz, Nov 25 2017

Issue description

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_ubsan_vptr_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  !CallbackFunction().IsEmpty() in v8_mutation_callback.cc
  blink::V8MutationCallback::Invoke
  blink::V8MutationCallback::InvokeAndReportException
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=519177:519178

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Nov 25 2017

Labels: OS-Windows OS-Mac
Project Member

Comment 2 by ClusterFuzz, Nov 25 2017

Labels: Test-Predator-Auto-Owner
Owner: yukishiino@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/07fbfbeb17a03c6872562bd8da226cacf023aafd (v8binding: Uses the safe v8::Context in callback functions.).

If this is incorrect, please remove the owner and apply the Test-Predator-Wrong-CLs label.
Project Member

Comment 3 by ClusterFuzz, Nov 26 2017

Labels: OS-Android
Status: Started (was: Assigned)
Working at
https://chromium-review.googlesource.com/c/chromium/src/+/799653
Cc: yukishiino@chromium.org
 Issue 788552  has been merged into this issue.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 30 2017

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

commit 9e4a1523e569b5774526d5f39f5c91270e7cf27c
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Thu Nov 30 11:40:58 2017

MutationObserver: Do nothing when its execution context is no longer available.

MutationObserver::DeliverMutations is a static function and
registered to the microtask queue.  It maintains active
observers and also makes active observers alive, but nothing
else (the underlying callback function nor execution context).

So, when DeliverMutations invokes each active observer,
DeliverMutations must make sure that each active observer's
execution context is still available.

This patch adds the check of the execution context.

Bug:  788495 , 789862
Change-Id: Ic350603cc8a7b13275adf042e731fdd9605373aa
Reviewed-on: https://chromium-review.googlesource.com/799653
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520509}
[add] https://crrev.com/9e4a1523e569b5774526d5f39f5c91270e7cf27c/third_party/WebKit/LayoutTests/fast/dom/MutationObserver/weak-callback-gc-crash-case2-expected.txt
[add] https://crrev.com/9e4a1523e569b5774526d5f39f5c91270e7cf27c/third_party/WebKit/LayoutTests/fast/dom/MutationObserver/weak-callback-gc-crash-case2.html
[modify] https://crrev.com/9e4a1523e569b5774526d5f39f5c91270e7cf27c/third_party/WebKit/Source/core/dom/MutationObserver.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 30 2017

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

commit 0b6352b3e8d9fb9e88fdb969c886204f5a36c64f
Author: Avi Drissman <avi@chromium.org>
Date: Thu Nov 30 21:00:32 2017

Revert "MutationObserver: Do nothing when its execution context is no longer available."

This reverts commit 9e4a1523e569b5774526d5f39f5c91270e7cf27c.

Reason for revert:

fast/dom/MutationObserver/weak-callback-gc-crash-case2.html fairly consistently fails on Win7 Tests (dbg)(1) . See https://ci.chromium.org/buildbot/chromium.win/Win7%20Tests%20%28dbg%29%281%29/64920 as an example.

Original change's description:
> MutationObserver: Do nothing when its execution context is no longer available.
> 
> MutationObserver::DeliverMutations is a static function and
> registered to the microtask queue.  It maintains active
> observers and also makes active observers alive, but nothing
> else (the underlying callback function nor execution context).
> 
> So, when DeliverMutations invokes each active observer,
> DeliverMutations must make sure that each active observer's
> execution context is still available.
> 
> This patch adds the check of the execution context.
> 
> Bug:  788495 , 789862
> Change-Id: Ic350603cc8a7b13275adf042e731fdd9605373aa
> Reviewed-on: https://chromium-review.googlesource.com/799653
> Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#520509}

TBR=peria@chromium.org,yukishiino@chromium.org,haraken@chromium.org

Change-Id: I00ea0349a3f441f6d2fb356d179b5e301c253df3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  788495 , 789862
Reviewed-on: https://chromium-review.googlesource.com/801222
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520680}
[delete] https://crrev.com/c6e0536bbdfb293ed22765f8892cd78e61232a03/third_party/WebKit/LayoutTests/fast/dom/MutationObserver/weak-callback-gc-crash-case2-expected.txt
[delete] https://crrev.com/c6e0536bbdfb293ed22765f8892cd78e61232a03/third_party/WebKit/LayoutTests/fast/dom/MutationObserver/weak-callback-gc-crash-case2.html
[modify] https://crrev.com/0b6352b3e8d9fb9e88fdb969c886204f5a36c64f/third_party/WebKit/Source/core/dom/MutationObserver.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 1 2017

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

commit e7849d0eaf4588a3cc9ca72b2cb8fbc7a68eb2e4
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Fri Dec 01 09:39:19 2017

MutationObserver: Do nothing when its execution context is no longer available. (reland)

This patch is a reland of https://crrev.com/c/799653 .
The fix is the same, but it dropped the layout test.
The test seems having a problem on Windows.
See also a revert at https://crrev.com/c/801222 .

MutationObserver::DeliverMutations is a static function and
registered to the microtask queue.  It maintains active
observers and also makes active observers alive, but nothing
else (the underlying callback function nor execution context).

So, when DeliverMutations invokes each active observer,
DeliverMutations must make sure that each active observer's
execution context is still available.

This patch adds the check of the execution context.

Bug:  788495 , 789862
Change-Id: I43ffe7eca0b62f16b3ad54c24d0aa8a5d0485348
Reviewed-on: https://chromium-review.googlesource.com/803155
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520897}
[modify] https://crrev.com/e7849d0eaf4588a3cc9ca72b2cb8fbc7a68eb2e4/third_party/WebKit/Source/core/dom/MutationObserver.cpp

Project Member

Comment 9 by ClusterFuzz, Dec 2 2017

ClusterFuzz has detected this issue as fixed in range 520889:520897.

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_ubsan_vptr_chrome
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  !CallbackFunction().IsEmpty() in v8_mutation_callback.cc
  blink::V8MutationCallback::Invoke
  blink::V8MutationCallback::InvokeAndReportException
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=519177:519178
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=520889:520897

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

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 10 by ClusterFuzz, Dec 2 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Started)
ClusterFuzz testcase 6356534065102848 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 11 by bugdroid1@chromium.org, Dec 5 2017

Labels: merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/86f4ba512932cca91cf5e287ba8593ad829974a0

commit 86f4ba512932cca91cf5e287ba8593ad829974a0
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Tue Dec 05 06:31:29 2017

MutationObserver: Do nothing when its execution context is no longer available. (reland)

This patch is a reland of https://crrev.com/c/799653 .
The fix is the same, but it dropped the layout test.
The test seems having a problem on Windows.
See also a revert at https://crrev.com/c/801222 .

MutationObserver::DeliverMutations is a static function and
registered to the microtask queue.  It maintains active
observers and also makes active observers alive, but nothing
else (the underlying callback function nor execution context).

So, when DeliverMutations invokes each active observer,
DeliverMutations must make sure that each active observer's
execution context is still available.

This patch adds the check of the execution context.

TBR=yukishiino@chromium.org

(cherry picked from commit e7849d0eaf4588a3cc9ca72b2cb8fbc7a68eb2e4)

Bug:  788495 , 789862
Change-Id: I43ffe7eca0b62f16b3ad54c24d0aa8a5d0485348
Reviewed-on: https://chromium-review.googlesource.com/803155
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#520897}
Reviewed-on: https://chromium-review.googlesource.com/807755
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#24}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/86f4ba512932cca91cf5e287ba8593ad829974a0/third_party/WebKit/Source/core/dom/MutationObserver.cpp

Sign in to add a comment