Bug 166879 - Calling async arrow function which is in a class's member function will cause error
Summary: Calling async arrow function which is in a class's member function will cause...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-09 23:12 PST by Shuan Zhao
Modified: 2017-05-21 14:08 PDT (History)
9 users (show)

See Also:


Attachments
Patch (8.64 KB, patch)
2017-01-10 06:48 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-09 23:12:50 PST
The test code is as follows. When I run the code, I got the error “Can't find private variable: @derivedConstructor”.

function doSomething(callback) {
    callback();
}

class Test {
    testFunc() {
        doSomething(async () => {
            console.log("testFunc");
            await sleep(2);
            console.log("testFunc end");
        });
    }
}

let test = new Test();
test.testFunc();
Comment 1 GSkachkov 2017-01-10 02:53:38 PST
I think root of the issue because property @derivedConstructor is not put to the virtual scope in testFunc. When we implemented simple arrow function we check if we super used inside and do put derivedConstructor to the virtual scope, before it it used within arrow function. 
So I think solution is adding additional check (generator.isSuperUsedInInnerArrowFunction()) in this line https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp#L3505 
If I'll try to prepare fix, but it will be later today.
Comment 2 GSkachkov 2017-01-10 06:48:45 PST
Created attachment 298461 [details]
Patch

Fix for the issue
Comment 3 WebKit Commit Bot 2017-01-10 13:14:36 PST
Comment on attachment 298461 [details]
Patch

Clearing flags on attachment: 298461

Committed r210558: <http://trac.webkit.org/changeset/210558>
Comment 4 WebKit Commit Bot 2017-01-10 13:14:40 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Shuan Zhao 2017-01-10 18:56:51 PST
I see this issue is solved, but there's another problem. I have updated the test code:

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();

Is it another bug? The code above is ok with V8.
Comment 6 GSkachkov 2017-01-11 00:02:04 PST
(In reply to comment #5)
> I see this issue is solved, but there's another problem. I have updated the
> test code:
> 
> 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();
> 
> Is it another bug? The code above is ok with V8.

Yeah, unfortunately it is another bug. Async arrow function 'lost' _this_, by some reason, during restore state after await.
Comment 7 JingAn Chen 2017-05-21 11:38:43 PDT
This bug is confirmed fixed in Safari Tech Preview as on May 21st, 2017. However, the current Safari macOS/iOS (10.1.1/10.3.2) still has it. Is there an ETA for the fix to be landed on normal Safari? In other words, how often does the push happens from Safari Tech Preview to Safari? Thanks!
Comment 8 Saam Barati 2017-05-21 14:08:18 PDT
(In reply to JingAn Chen from comment #7)
> This bug is confirmed fixed in Safari Tech Preview as on May 21st, 2017.
> However, the current Safari macOS/iOS (10.1.1/10.3.2) still has it. Is there
> an ETA for the fix to be landed on normal Safari? In other words, how often
> does the push happens from Safari Tech Preview to Safari? Thanks!

This is the WebKit bug tracker. We don't comment on Safari release plans here.