Bug 166919 - "this" missing after await in async arrow function
Summary: "this" missing after await in async arrow function
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-10 23:59 PST by Shuan Zhao
Modified: 2017-01-21 03:38 PST (History)
8 users (show)

See Also:


Attachments
Patch (7.43 KB, patch)
2017-01-11 06:20 PST, GSkachkov
no flags Details | Formatted Diff | Diff
Patch (7.56 KB, patch)
2017-01-11 23:55 PST, GSkachkov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shuan Zhao 2017-01-10 23:59:47 PST
My test code is as follows.

function runSomething(callback) {
	callback();
}

class Test {
	testFunc() {
		this.prop = "123";
		runSomething(async () => {
			console.log("callback " + this.prop); // It's ok here
			await sleep(2);
			console.log("callback end " + this.prop); // TypeError: undefined is not an object (evaluating 'this.prop')
		});
	}
}

let t = new Test();
t.testFunc();
Comment 1 GSkachkov 2017-01-11 04:01:37 PST
Hmm, it seems that find route of issue. 
At start execution of arrow function we load _this_ from virtual scope and put it to _this_ register, but when we suspend async arrow function and resume after await, we resume with value from generator.@generatorThis, that is empty. I'll try to prepare fix during today.
Comment 2 GSkachkov 2017-01-11 06:20:08 PST
Created attachment 298574 [details]
Patch

patch
Comment 3 GSkachkov 2017-01-11 23:55:06 PST
Created attachment 298665 [details]
Patch

Update readme
Comment 4 Saam Barati 2017-01-15 12:36:17 PST
Comment on attachment 298665 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=298665&action=review

r=me

> Source/JavaScriptCore/ChangeLog:10
> +        Async arrow function can be suspende and during resuming shold be use _this_ from 

"suspende" => "suspended"
"and during resuming shold be use _this_" => "and when resumed should use _this_"

> JSTests/stress/async-arrow-functions-lexical-binding-in-class.js:52
>      asyncThisPropBody() {

Can you also add some tests where we bind various "this" values and make sure we get the correct result.
Comment 5 GSkachkov 2017-01-19 10:35:26 PST
All reviewed patches have been landed.
Committed r210925: <http://trac.webkit.org/changeset/210925>
Comment 6 GSkachkov 2017-01-20 11:57:48 PST
Comment on attachment 298665 [details]
Patch

Clearing flags on attachment: 298665

Patch landed
Comment 7 GSkachkov 2017-01-20 12:02:11 PST
(In reply to comment #0)
> My test code is as follows.
> 
> function runSomething(callback) {
> 	callback();
> }
> 
> class Test {
> 	testFunc() {
> 		this.prop = "123";
> 		runSomething(async () => {
> 			console.log("callback " + this.prop); // It's ok here
> 			await sleep(2);
> 			console.log("callback end " + this.prop); // TypeError: undefined is not
> an object (evaluating 'this.prop')
> 		});
> 	}
> }
> 
> let t = new Test();
> t.testFunc();

Shuan Zhao, Could you please check if issue is fixed and close it? 
I don't have permissions to do this.
Comment 8 GSkachkov 2017-01-21 03:38:06 PST
Shuan Zhao, Thanks for filling and closing issue :-)!
Saam Barati, Thanks for review!