Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fxIsFunction gives wrong result for revoked proxy #297

Closed
devsnek opened this issue Nov 29, 2019 · 3 comments
Closed

fxIsFunction gives wrong result for revoked proxy #297

devsnek opened this issue Nov 29, 2019 · 3 comments

Comments

@devsnek
Copy link

devsnek commented Nov 29, 2019

const p = Proxy.revocable(function(){}, {});
p.revoke();
print(typeof p.proxy);

should be function, but xs gives object. This happens because xsIsFunction redirects the lookup to proxy.target, but proxy.target is nulled out when the proxy is revoked:

txBoolean fxIsFunction(txMachine* the, txSlot* instance)
{
again:
if (instance) {
txSlot* exotic = instance->next;
if (exotic && (exotic->flag & XS_INTERNAL_FLAG)) {
if (((exotic->kind == XS_CALLBACK_KIND) || (exotic->kind == XS_CALLBACK_X_KIND) || (exotic->kind == XS_CODE_KIND) || (exotic->kind == XS_CODE_X_KIND)))
return 1;
if (exotic->kind == XS_PROXY_KIND) {
instance = exotic->value.proxy.target;
goto again;
}
}
}
return 0;
}

I'm not sure how to run js files with the output of the build, but i did come up with this patch:

diff --git a/xs/sources/xsAll.h b/xs/sources/xsAll.h
index 31deeee4..83b2785d 100644
--- a/xs/sources/xsAll.h
+++ b/xs/sources/xsAll.h
@@ -1718,6 +1718,7 @@ enum {
        XS_LEVEL_FLAG = 32,
        XS_DONT_MARSHALL_FLAG = 64,
        /* XS_MARK_FLAG = 128, */
+       XS_PROXY_IS_CALLABLE_FLAG = 256,
 
        /* property flags */
        XS_INTERNAL_FLAG = 1,
diff --git a/xs/sources/xsFunction.c b/xs/sources/xsFunction.c
index b5df3fc5..89f2c4e9 100644
--- a/xs/sources/xsFunction.c
+++ b/xs/sources/xsFunction.c
@@ -113,15 +113,13 @@ txBoolean fxIsCallable(txMachine* the, txSlot* slot)
 
 txBoolean fxIsFunction(txMachine* the, txSlot* instance)
 {
-again:
        if (instance) {
                txSlot* exotic = instance->next;
                if (exotic && (exotic->flag & XS_INTERNAL_FLAG)) {
                        if (((exotic->kind == XS_CALLBACK_KIND) || (exotic->kind == XS_CALLBACK_X_KIND) || (exotic->kind == XS_CODE_KIND) || (exotic->kind == XS_CODE_X_KIND)))
                                return 1;
                        if (exotic->kind == XS_PROXY_KIND) {
-                               instance = exotic->value.proxy.target;
-                               goto again;
+                               return exotic->flag & XS_PROXY_IS_CALLABLE_FLAG;
                        }
                }
        }
diff --git a/xs/sources/xsProxy.c b/xs/sources/xsProxy.c
index 42017094..4227457d 100644
--- a/xs/sources/xsProxy.c
+++ b/xs/sources/xsProxy.c
@@ -825,6 +825,9 @@ void fx_Proxy(txMachine* the)
        if ((handler->next) && (handler->next->kind == XS_PROXY_KIND) && !handler->next->value.proxy.handler)
                mxTypeError("handler is a revoked proxy");
        instance->flag |= target->flag & XS_CAN_CONSTRUCT_FLAG;
+       if (fxIsCallable(the, target)) {
+               instance->flag |= XS_PROXY_IS_CALLABLE_FLAG;
+       }
        proxy->value.proxy.target = target;
        proxy->value.proxy.handler = handler;
 }
@@ -854,6 +857,10 @@ void fx_Proxy_revocable(txMachine* the)
        mxPushUndefined();
        instance = fxNewProxyInstance(the);
        slot = instance->next;
+       instance->flag |= target->flag & XS_CAN_CONSTRUCT_FLAG;
+       if (fxIsCallable(the, target)) {
+               instance->flag |= XS_PROXY_IS_CALLABLE_FLAG;
+       }
        slot->value.proxy.target = target;
        slot->value.proxy.handler = handler;
        property = fxNextSlotProperty(the, property, the->stack, mxID(_proxy), XS_GET_ONLY);
@devsnek
Copy link
Author

devsnek commented Jan 16, 2020

bump

@phoddie
Copy link
Collaborator

phoddie commented May 21, 2020

The patch won't work because instance->flag is a byte so the flag value of 256 will be lost. To minimize memory use, critical on microcontrollers targets, it is impractical to increase the size of that field.

I just ran your example with the latest Moddable SDK and the output is function. Are you still able to reproduce the problem?

@devsnek
Copy link
Author

devsnek commented May 21, 2020

This does appear to have been fixed at some point.

@devsnek devsnek closed this as completed May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants