saghul / txiki.js

A tiny JavaScript runtime
MIT License
2.55k stars 171 forks source link

ffi string not working as expected #653

Closed KaruroChori closed 1 month ago

KaruroChori commented 1 month ago
//import type from '@txikijs/types';

const sopath = './libcfltk.so';
import FFI from 'tjs:ffi';
const testlib = new FFI.Lib(sopath);

const cfltk ={
    Fl_Window_new: new FFI.CFunction(testlib.symbol('Fl_Window_new'), FFI.types.pointer, [FFI.types.sint,FFI.types.sint,FFI.types.sint,FFI.types.sint,FFI.types.string]),
    Fl_Window_end: new FFI.CFunction(testlib.symbol('Fl_Window_end'), FFI.types.void, [FFI.types.pointer]),
    Fl_Window_show: new FFI.CFunction(testlib.symbol('Fl_Window_show'), FFI.types.void, [FFI.types.pointer]),
    Fl_run: new FFI.CFunction(testlib.symbol('Fl_run'),FFI.types.void, [])
}

const handle = cfltk.Fl_Window_new.call(0, 0, 300, 400, "AAAAA");
cfltk.Fl_Window_end.call(handle);
cfltk.Fl_Window_show.call(handle);
cfltk.Fl_run.call();

The window should show "AAAAA" as title. The same (adapted) code run in Bun results in the expected behaviour. According to code I have seen in test this should be valid.

I attached the library in case you want to try it youself. It is derived from https://github.com/MoAlyousef/cfltk for linux amd64. libcfltk.so.gz

KaruroChori commented 1 month ago

Surprisingly, testing it on a different library is fine:

const testlib2 = new FFI.Lib("./libsum.so");
const lib2={
    concatenateStrings: new FFI.CFunction(testlib2.symbol('concatenateStrings'), FFI.types.string, [FFI.types.string,FFI.types.string])
}
console.log(lib2.concatenateStrings.call("h","e"))

results in the expected "he".

And with this

//import type from '@txikijs/types';

const sopath = './libcfltk.so';
import FFI from 'tjs:ffi';
const testlib = new FFI.Lib(sopath);

const testlib2 = new FFI.Lib("./libsum.so");
const lib2={
    concatenateStrings: new FFI.CFunction(testlib2.symbol('concatenateStrings'), FFI.types.pointer, [FFI.types.string,FFI.types.string])
}
console.log(lib2.concatenateStrings.call("h","e"))

const cfltk = {
    Fl_Window_new: new FFI.CFunction(testlib.symbol('Fl_Window_new'), FFI.types.pointer, [FFI.types.sint32,FFI.types.sint32,FFI.types.sint32,FFI.types.sint32,FFI.types.pointer]),
    Fl_Window_end: new FFI.CFunction(testlib.symbol('Fl_Window_end'), FFI.types.void, [FFI.types.pointer]),
    Fl_Window_show: new FFI.CFunction(testlib.symbol('Fl_Window_show'), FFI.types.void, [FFI.types.pointer]),
    Fl_run: new FFI.CFunction(testlib.symbol('Fl_run'),FFI.types.void, [])
}

const handle = cfltk.Fl_Window_new.call(0, 0, 300, 400, lib2.concatenateStrings.call("h","e"));
cfltk.Fl_Window_end.call(handle);
cfltk.Fl_Window_show.call(handle);
cfltk.Fl_run.call();

The window has title "he" (notice they are no longer strings but pointers. Otherwise it does not work)

KaruroChori commented 1 month ago

It looks like a bug related to the lifetime of that string (the prototype is using pointer in place of string here). This works:

const vuff=(new TextEncoder()).encode('hello\0');
const handle = cfltk.Fl_Window_new.call(0, 0, 300, 400,ffiInt.getArrayBufPtr(vuff) );

This does not:

const handle = cfltk.Fl_Window_new.call(0, 0, 300, 400,ffiInt.getArrayBufPtr((new TextEncoder()).encode('hello\0')) );
KaruroChori commented 1 month ago

Me stupid. It clearly cannot work, as that function expects the string to outlast the actual time it will be rendering as no copy is internally performed. The semantic of .call was hiding that internally a UInt8Array is generated from the decoded string, and its lifetime is not the same as for the original string.

In bun it worked just out of luck because the gc is delayed, while in txiki the reference counting mechanism removes that object as soon as it is out of scope.

In short, the string helpers provided by tjs:ffi cannot be used as they are if the C code on the other side is expecting to take ownership of the buffer being passed or has expectations over its lifetime.

My feeling is that string as an AdvancedType in the ffi module is intrinsically broken as the buffer must exist outside that narrow scope. Something like an explicit CString class we can use.