Issue 734633004: Fix WaitingCallback to not run JavaScript during GC

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 734633004: Fix WaitingCallback to not run JavaScript during GC (Closed)

Created:
6 years, 1 month ago by eseidel

Modified:
6 years, 1 month ago

Reviewers:
hansmuller, abarth-chromium

CC:
abarth-chromium, Aaron Boodman, ben+mojo_chromium.org, darin (slow to review), mojo-reviews_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org

Base URL:
git@github.com:domokit/mojo.git@master

Project:
mojo

Visibility:
Public.

More Reviews

Description

Fix WaitingCallback to not run JavaScript during GC We were occasionally hitting an ASSERT in sky, abarth and I finally tracked it down this afternoon to WaitingCallback and HandleWrapper causing JS execution during GC. This fixes WaitingCallback to immediately clear its wait handle on destruction but wait to notify JS clients until outside of GC. Pure-js based mojo was undoubtedly hitting this as well, it just didn't have the same ASSERTs from blink to catch this. R=abarth@chromium.org, hansmuller@chromium.org BUG=434109 Committed: https://chromium.googlesource.com/external/mojo/+/6d1671e3edafe3314571c8dbab9e486d62f20cd4

Patch Set 1 #

Created: 6 years, 1 month ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+29 lines, -4 lines)			Patch
	M	mojo/edk/js/waiting_callback.h	View		2 chunks	+5 lines, -0 lines	0 comments	Download
	M	mojo/edk/js/waiting_callback.cc	View		4 chunks	+24 lines, -4 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

abarth-chromium

LGTM Another option should be to keep a strong handle to the callback while we're ...

6 years, 1 month ago (2014-11-18 01:51:18 UTC) #2

eseidel

It's not clear to me which behavior is correct. But I'm landing this to get ...

6 years, 1 month ago (2014-11-18 19:14:29 UTC) #3

eseidel

6 years, 1 month ago (2014-11-18 19:14:42 UTC) #4

Message was sent while issue was closed.

Committed patchset #1 (id:1) manually as
6d1671e3edafe3314571c8dbab9e486d62f20cd4 (presubmit successful).

Expand Messages | Collapse Messages