Bug 197828 - [JSC] Shouldn't drain the microtask queue before call native extend obj registerd by use api (cause error in Promise define)
Summary: [JSC] Shouldn't drain the microtask queue before call native extend obj regis...
Status: RESOLVED DUPLICATE of bug 161942
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Other
Hardware: All macOS 10.14
: P2 Critical
Assignee: Keith Miller
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-12 22:46 PDT by chen
Modified: 2021-06-07 19:35 PDT (History)
10 users (show)

See Also:


Attachments
Patch (5.65 KB, patch)
2021-06-07 19:33 PDT, Keith Miller
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chen 2019-05-12 22:46:16 PDT
Shouldn't drain the microtask queue before call native extend obj registerd by use api (cause error in Promise define)

our project blocked by this issue.

search buglist ,find similar [Shouldn't drain the micro task queue when calling out to ObjC](https://bugs.webkit.org/show_bug.cgi?id=161929#c3) ,but uesless for in this case.


test case 

```
//console.log() is native extned function register by api

var promise1 = new Promise(function(resolve, reject) {
 resolve("");
});

promise1.then(function(value) {
  console.log("--> 2");
});

console.log("--> 1"); 
```

expect result (right,excute script in Safari 12.0)

```
---> 1
---> 2
```

but in fact ,excute reult (wrong)

```
----> 2
----> 1
```

with [Promise stand](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise) define, result should be case 1.


---

native extend code : 

```
//register console.log(msg) to globalObject

void Manager::addConsoleObj() {
    JSClassDefinition console_definition = kJSClassDefinitionEmpty;
    console_definition.staticFunctions = evilStaticFunctions;

    JSClassRef classRef = JSClassCreate(&console_definition);
    JSObjectRef consoleObj = JSObjectMake(ctx_, classRef, nullptr);
    JSClassRelease(classRef);


    JSStringRef consoleStringName = JSStringCreateWithUTF8CString("console");
    JSObjectSetProperty(ctx_,
                        JSContextGetGlobalObject(ctx_),
                        consoleStringName,
                        consoleObj,
                        kJSPropertyAttributeNone,
                        nullptr);
    JSStringRelease(consoleStringName);
}


Console_log(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount,
            const JSValueRef arguments[],
            JSValueRef *exception) {
 	//print log
}

```
---

__we read the source code ,find issue here__

```
EncodedJSValue JSC_HOST_CALL APICallbackFunction::call(ExecState* exec)
{
    JSValueRef result;
    {
    	 //bug in dropAllLocks
    	 JSLock::DropAllLocks dropAllLocks(exec);
    	 
        result = jsCast<T*>(toJS(functionRef))->functionCallback()(execRef, functionRef, thisObjRef, argumentCount, arguments.data(), &exception);
    }
}

```

`JSLock::DropAllLocks` will unlock all vm lockcount ,then call `willReleaseLock` to drainMicrotasks(quene has promise.then task `console.log("---> 2")` ).


func called sequence

```
-> APICallbackFunction::call
-> dropAllLocks(exec) 
-> dropAllLocks(DropAllLocks* dropper)
-> unlock(droppedLockCount)
-> willReleaseLock()
-> vm->drainMicrotasks()
```


Looking forward to your reply
Comment 1 chen 2019-05-15 23:07:30 PDT
same bug here https://bugs.webkit.org/show_bug.cgi?id=161942 in 2016, and haven't fix after 3 years....
Comment 2 chyizheng 2021-04-19 22:54:52 PDT
+1
Comment 3 Yusuke Suzuki 2021-04-20 00:15:42 PDT

*** This bug has been marked as a duplicate of bug 161942 ***
Comment 4 Keith Miller 2021-06-07 19:33:56 PDT
Reopening to attach new patch.
Comment 5 Keith Miller 2021-06-07 19:33:57 PDT
Created attachment 430800 [details]
Patch
Comment 6 Keith Miller 2021-06-07 19:35:04 PDT
Whoops, attached this to the wrong bug.

*** This bug has been marked as a duplicate of bug 161942 ***