wren-lang / wren

The Wren Programming Language. Wren is a small, fast, class-based concurrent scripting language.
http://wren.io
MIT License
6.9k stars 552 forks source link

"Cannot already be in foreign call." error when using Debug build only #690

Open mattj1 opened 5 years ago

mattj1 commented 5 years ago

Hey, I'm new to Wren, and while trying to update IronWren to work with the latest Wren source, I noticed that some of the example code demonstrating the use of the Wren API was failing, but only when using the Debug library. I re-wrote the code in C to see if it was an issue with C# or whatever, but it turns out I could re-produce the problem:

#include <stdio.h>
#include <string.h>
#include "wren.h"

const char *module = "main";

void wren_write(WrenVM *vm, const char *text) {
    printf("%s", text);
}

void wren_error(WrenVM *vm, WrenErrorType type, const char *module, int line, const char *message) {
    printf("Error: %d: %s\n", line, message);
}

void sayHi(WrenVM *vm) {

    const char *str = wrenGetSlotString(vm, 1);
    printf("sayHi: %s!\n", str);
}

static void alloc(WrenVM *vm) {
    void *obj = wrenSetSlotNewForeign(vm, 0, 1, sizeof(int));
}

static WrenForeignMethodFn bindForeignMethod(WrenVM *vm,
        const char *module, const char *className, bool isStatic,
        const char *signature) {

    printf("BindForeignMethod called: It's called %s, part of %s, isStatic: %d\n", signature, className, isStatic);

    if (!strcmp(signature, "sayHi(_)")) {
        return sayHi;
    }

    return NULL;
}

static WrenForeignClassMethods bindForeignClass(WrenVM *vm, const char *module, const char *className) {
    printf("bindForeignClass called: It's called %s\n", className);

    WrenForeignClassMethods methods = {NULL, NULL};
    if (!strcmp(className, "Test")) {
        methods.allocate = alloc;
    }
    return methods;
}

int main(int argc, const char *argv[]) {

    WrenConfiguration config;
    wrenInitConfiguration(&config);
    config.writeFn = wren_write;
    config.errorFn = wren_error;
    config.bindForeignMethodFn = bindForeignMethod;
    config.bindForeignClassFn = bindForeignClass;

    WrenVM *vm = wrenNewVM(&config);

    wrenInterpret(vm, module, "var helloTo = Fn.new { |name|\n"
                                   "System.print(\"Hello, %(name)!\")\n"
                                   "}");

    wrenInterpret(vm, module, "helloTo.call(\"foo\")");

    WrenHandle *someFnHandle = wrenMakeCallHandle(vm, "call(_)");
    wrenEnsureSlots(vm, 2);
    wrenGetVariable(vm, module, "helloTo", 0);
    wrenSetSlotString(vm, 1, "bar");
    wrenCall(vm, someFnHandle);
    wrenReleaseHandle(vm, someFnHandle);

    const char *source = "foreign class Test {\n"
                         "\tconstruct new() { }\n"
                         "\tisForeign { true }\n"
                         "\tforeign sayHi(to)\n"
                         "}\n"
                         "var test = Test.new()\n"
                         "test.sayHi(\"wren\")\n";

    // Error: When linking to a debug build of wren:
    // [src/vm/wren_vm.c:627] Assert failed in createForeign(): Cannot already be in foreign call.
    wrenInterpret(vm, module, source);

    wrenFreeVM(vm);
    return 0;
}
mhermier commented 5 years ago

I don't understand the exact details, but it seems you achieved reentrancy somehow, and this is not supported by wren.

bjorn commented 5 years ago

Well, the assertion thinks this is happening, but if you look at the code it's not, so this is a bug.

I think it's due to the following code in wrenCall, which leaves vm->apiStack pointing somewhere:

https://github.com/wren-lang/wren/blob/d1a0d0682ac072fa20f2dcca356dac06565e93a1/src/vm/wren_vm.c#L1394-L1399

vm->apiStack seems to be only set to NULL by wrenCall itself, but the above program calls wrenInterpret after the wrenCall. That means that on the first call to a foreign method, it runs into the assertion even though in this case vm->apiStack not being NULL did not mean it was already in a foreign call.

Either a different method must be used to allow access the call's return value, or there needs to be another place where vm->apiStack is reset to NULL (or the assertion needs to be changed).

mhermier commented 5 years ago

There are other odd cases, related to io/libuv. A stack-trace could help to understand better what is going on.

mattj1 commented 5 years ago

Here's my stack trace:

__pthread_kill 0x00007fff7d65c2c6
pthread_kill 0x00007fff7d717bf1
abort 0x00007fff7d5c66a6
createForeign wren_vm.c:627
runInterpreter wren_vm.c:1200
wrenInterpret wren_vm.c:1449
main main.c:82
start 0x00007fff7d5213d5
start 0x00007fff7d5213d5
mhermier commented 5 years ago

Stack trace looks like a corruptions/bug. Someone should try to reproduce with valgrind.