New issue
Advanced search Search tips

Issue 798500 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 161072



Sign in to add a comment

Web Locks API: Lock released if waiting promise is GC'd

Project Member Reported by jsb...@chromium.org, Jan 2 2018

Issue description

Noted in  issue 161072  #c25:

navigator.locks.acquire( 'mylock', function(lock) {
	console.log('acquired lock', performance.now());
	return new Promise(function(resolve){});
}).then( function(e) {
	console.log('released lock', e, performance.now())
});

Running this twice, the lock gets acquired after GC reclaims the Promise. The Lock instance is being kept alive only by the Promise->ThenFunction->Lock membership chain. This is insufficient here - the Lock should live until the context is destroyed.



 
Blocking: 161072
With a related test case in debug builds we hit:

FATAL:ScriptPromiseResolver.h(57)] Check failed: state_ == kDetached || !is_promise_called_ || !GetScriptState()->ContextIsValid() || !GetExecutionContext() || GetExecutionContext()->IsContextDestroyed(). 


Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 6 2018

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

commit e3475d84bc72d9c5eaf47f188a01bd0ac9495d0a
Author: Joshua Bell <jsbell@chromium.org>
Date: Sat Jan 06 04:35:07 2018

Web Locks API: Ensure never-resolved promises hold locks forever

If a lock is granted and a never-resolved promise is returned to
control the lock's lifetime, the lock should be held until the context
is destroyed (i.e. lock lifetime should match page lifetime).

This requires special handling when neither the lock nor the promise
is retained by script, to prevent GC from destroying the lock wrapper
and releasing the lock. To do so, have the LockManager (which has the
same lifetime as the execution context) maintain a set of unresolved
locks.

Bug:  798500 
Change-Id: I4086349c45601489328e614ae4b55ecf2c1a3af4
Reviewed-on: https://chromium-review.googlesource.com/848295
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527506}
[add] https://crrev.com/e3475d84bc72d9c5eaf47f188a01bd0ac9495d0a/third_party/WebKit/LayoutTests/http/tests/locks/chromium-waiting-promise-gc.html
[modify] https://crrev.com/e3475d84bc72d9c5eaf47f188a01bd0ac9495d0a/third_party/WebKit/LayoutTests/http/tests/locks/frames.html
[modify] https://crrev.com/e3475d84bc72d9c5eaf47f188a01bd0ac9495d0a/third_party/WebKit/LayoutTests/http/tests/locks/query.html
[add] https://crrev.com/e3475d84bc72d9c5eaf47f188a01bd0ac9495d0a/third_party/WebKit/LayoutTests/http/tests/locks/resources/helpers.js
[modify] https://crrev.com/e3475d84bc72d9c5eaf47f188a01bd0ac9495d0a/third_party/WebKit/LayoutTests/http/tests/locks/workers.html
[modify] https://crrev.com/e3475d84bc72d9c5eaf47f188a01bd0ac9495d0a/third_party/WebKit/Source/modules/locks/Lock.cpp
[modify] https://crrev.com/e3475d84bc72d9c5eaf47f188a01bd0ac9495d0a/third_party/WebKit/Source/modules/locks/Lock.h
[modify] https://crrev.com/e3475d84bc72d9c5eaf47f188a01bd0ac9495d0a/third_party/WebKit/Source/modules/locks/LockManager.cpp
[modify] https://crrev.com/e3475d84bc72d9c5eaf47f188a01bd0ac9495d0a/third_party/WebKit/Source/modules/locks/LockManager.h

Status: Fixed (was: Started)

Sign in to add a comment