Skip to content

Commit 7f40363

Browse files
authored
Merge pull request #21 from fastschema/fix.isolate_option_per_runtime
fix: prevent shared Option causing race by switching to value instead of pointer
2 parents 7c123fc + 903b8b5 commit 7f40363

File tree

13 files changed

+267
-151
lines changed

13 files changed

+267
-151
lines changed

Makefile

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
.PHONY: build clean
1+
.PHONY: build build-debug clean
22

33
build:
44
@echo "Configuring and building qjs..."
@@ -16,6 +16,23 @@ build:
1616

1717
wasm-opt -O3 qjs.wasm -o qjs.wasm
1818

19+
build-debug:
20+
@echo "Configuring and building qjs with runtime address debug..."
21+
cd qjswasm/quickjs && \
22+
rm -rf build && \
23+
cmake -B build \
24+
-DQJS_BUILD_LIBC=ON \
25+
-DQJS_BUILD_CLI_WITH_MIMALLOC=OFF \
26+
-DQJS_DEBUG_RUNTIME_ADDRESS=ON \
27+
-DCMAKE_TOOLCHAIN_FILE=/opt/wasi-sdk/share/cmake/wasi-sdk.cmake \
28+
-DCMAKE_PROJECT_INCLUDE=../qjswasm.cmake
29+
@echo "Building qjs target..."
30+
make -C qjswasm/quickjs/build qjswasm -j$(nproc)
31+
@echo "Copying build/qjswasm to top-level as qjs.wasm..."
32+
cp qjswasm/quickjs/build/qjswasm qjs.wasm
33+
34+
wasm-opt -O3 qjs.wasm -o qjs.wasm
35+
1936
clean:
2037
@echo "Cleaning build directory..."
2138
cd quickjs && rm -rf build

options.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,16 +213,16 @@ func (o *EvalOption) Free() {
213213
}
214214
}
215215

216-
func getRuntimeOption(registry *ProxyRegistry, options ...*Option) (option *Option, err error) {
217-
if len(options) == 0 || options[0] == nil {
218-
option = &Option{}
216+
func getRuntimeOption(registry *ProxyRegistry, options ...Option) (option Option, err error) {
217+
if len(options) == 0 {
218+
option = Option{}
219219
} else {
220220
option = options[0]
221221
}
222222

223223
if option.CWD == "" {
224224
if option.CWD, err = os.Getwd(); err != nil {
225-
return nil, fmt.Errorf("cannot get current working directory: %w", err)
225+
return Option{}, fmt.Errorf("cannot get current working directory: %w", err)
226226
}
227227
}
228228

options_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,7 @@ func TestEvalOptions(t *testing.T) {
3939

4040
t.Run("explicit_cwd_provided", func(t *testing.T) {
4141
tempDir := t.TempDir()
42-
options := &qjs.Option{
43-
CWD: tempDir,
44-
}
45-
46-
runtime, err := qjs.New(options)
42+
runtime, err := qjs.New(qjs.Option{CWD: tempDir})
4743
require.NoError(t, err)
4844
runtime.Close()
4945
})

proxy.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,6 @@ func createFuncProxyWithRegistry(registry *ProxyRegistry) JsFunctionProxy {
125125
) (rs uint64) {
126126
goFunc, this := getProxyFuncParams(registry, module.Memory(), thisVal, argc, argv)
127127

128-
// Ensure panic recovery for safe execution
129128
defer func() {
130129
if r := recover(); r != nil {
131130
rs = handlePanicRecovery(this, r)
@@ -182,7 +181,6 @@ func getProxyFuncParams(
182181
isAsync := args[2] // The third argument is the async flag
183182

184183
goFunc, goContext := retrieveGoResources(registry, functionHandle, jsContextHandle)
185-
186184
promise := extractPromiseIfAsync(goContext, args, isAsync)
187185
fnArgs := extractFunctionArguments(goContext, args)
188186
this := createThisContext(goContext, thisRef, fnArgs, promise, isAsync)

qjs.wasm

8 Bytes
Binary file not shown.

qjswasm/helpers.c

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,37 @@
33
#include <limits.h>
44
#include <float.h>
55

6+
#ifdef QJS_DEBUG_RUNTIME_ADDRESS
7+
/**
8+
* Allocates a random-sized block of memory to randomize address space layout.
9+
* This helps in debugging by ensuring runtime objects are allocated at different
10+
* addresses on each run, making pointer-related bugs more apparent.
11+
*/
12+
void randomize_address_space(void)
13+
{
14+
static unsigned int call_counter = 0;
15+
int stack_variable;
16+
17+
// Generate entropy from multiple sources for better randomization
18+
unsigned int entropy = (unsigned int)((uintptr_t)&stack_variable ^ // Stack address (varies per call)
19+
(uintptr_t)time(NULL) ^ // Current time
20+
(uintptr_t)clock() ^ // Clock ticks (higher resolution)
21+
(++call_counter) // Incremental counter
22+
);
23+
24+
// Allocate 1-1024 bytes to perturb the address space
25+
size_t allocation_size = (entropy % 1024) + 1;
26+
volatile void *random_allocation = malloc(allocation_size);
27+
28+
// Note: Intentionally not freeing to affect subsequent allocations
29+
// Touch the allocated memory to ensure it's not optimized away
30+
if (random_allocation)
31+
{
32+
*((volatile char *)random_allocation) = 0;
33+
}
34+
}
35+
#endif
36+
637
JSValue JS_NewNull() { return JS_NULL; }
738
JSValue JS_NewUndefined() { return JS_UNDEFINED; }
839
JSValue JS_NewUninitialized() { return JS_UNINITIALIZED; }
@@ -606,34 +637,8 @@ JSValue QJS_Call(JSContext *ctx, JSValue func, JSValue this, int argc, uint64_t
606637
return JS_Call(ctx, func, this, argc, js_argv);
607638
}
608639

609-
// Create a new QJS_PROXY_VALUE instance directly in C for better performance
610-
JSValue QJS_NewProxyValue(JSContext *ctx, int64_t proxyId)
640+
void QJS_Panic()
611641
{
612-
// Get the QJS_PROXY_VALUE constructor from global object
613-
JSValue global_obj = JS_GetGlobalObject(ctx);
614-
JSValue ctor = JS_GetPropertyStr(ctx, global_obj, "QJS_PROXY_VALUE");
615-
JS_FreeValue(ctx, global_obj);
616-
617-
if (JS_IsException(ctor) || JS_IsUndefined(ctor)) {
618-
JS_FreeValue(ctx, ctor);
619-
return JS_ThrowReferenceError(ctx, "QJS_PROXY_VALUE is not defined");
620-
}
621-
622-
// Create argument for the constructor (proxyId)
623-
JSValue arg = JS_NewInt64(ctx, proxyId);
624-
JSValue args[1] = { arg };
625-
626-
// Call the constructor with 'new'
627-
JSValue result = JS_CallConstructor(ctx, ctor, 1, args);
628-
629-
// Clean up
630-
JS_FreeValue(ctx, ctor);
631-
JS_FreeValue(ctx, arg);
632-
633-
return result;
634-
}
635-
636-
void QJS_Panic() {
637642
// Handle panic situation
638643
fprintf(stderr, "QJS Panic: Unrecoverable error occurred\n");
639644
abort();

qjswasm/proxy.c

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
#include "qjs.h"
2+
3+
// toString method for QJS_PROXY_VALUE class
4+
static JSValue qjs_proxy_value_toString(JSContext *ctx, JSValueConst this_val, int argc, JSValueConst *argv)
5+
{
6+
JSValue proxy_id = JS_GetPropertyStr(ctx, this_val, "proxyId");
7+
if (JS_IsException(proxy_id))
8+
return proxy_id;
9+
10+
const char *proxy_id_str = JS_ToCString(ctx, proxy_id);
11+
JS_FreeValue(ctx, proxy_id);
12+
13+
if (!proxy_id_str)
14+
return JS_EXCEPTION;
15+
16+
char buffer[256];
17+
snprintf(buffer, sizeof(buffer), "[object QJS_PROXY_VALUE(proxyId: %s)]", proxy_id_str);
18+
JS_FreeCString(ctx, proxy_id_str);
19+
20+
return JS_NewString(ctx, buffer);
21+
}
22+
23+
// Constructor function for QJS_PROXY_VALUE class
24+
static JSValue qjs_proxy_value_constructor(JSContext *ctx, JSValueConst new_target, int argc, JSValueConst *argv)
25+
{
26+
JSValue obj;
27+
JSValue proto;
28+
29+
if (JS_IsUndefined(new_target))
30+
{
31+
// Called as function, not constructor
32+
return JS_ThrowTypeError(ctx, "QJS_PROXY_VALUE must be called with new");
33+
}
34+
35+
// Get prototype from new_target
36+
proto = JS_GetPropertyStr(ctx, new_target, "prototype");
37+
if (JS_IsException(proto))
38+
return proto;
39+
40+
// Create object with proper prototype
41+
obj = JS_NewObjectProto(ctx, proto);
42+
JS_FreeValue(ctx, proto);
43+
44+
if (JS_IsException(obj))
45+
return obj;
46+
47+
// Set the proxyId property
48+
if (argc > 0)
49+
{
50+
if (JS_SetPropertyStr(ctx, obj, "proxyId", JS_DupValue(ctx, argv[0])) < 0)
51+
{
52+
JS_FreeValue(ctx, obj);
53+
return JS_EXCEPTION;
54+
}
55+
}
56+
else
57+
{
58+
if (JS_SetPropertyStr(ctx, obj, "proxyId", JS_UNDEFINED) < 0)
59+
{
60+
JS_FreeValue(ctx, obj);
61+
return JS_EXCEPTION;
62+
}
63+
}
64+
65+
return obj;
66+
}
67+
68+
// Initialize QJS_PROXY_VALUE class and add it to global object
69+
int init_qjs_proxy_value_class(JSContext *ctx)
70+
{
71+
JSValue global_obj = JS_GetGlobalObject(ctx);
72+
73+
// Create prototype object with toString method
74+
JSValue proto = JS_NewObject(ctx);
75+
JSValue toString_func = JS_NewCFunction(ctx, qjs_proxy_value_toString, "toString", 0);
76+
if (JS_SetPropertyStr(ctx, proto, "toString", toString_func) < 0)
77+
{
78+
JS_FreeValue(ctx, proto);
79+
JS_FreeValue(ctx, global_obj);
80+
return -1;
81+
}
82+
83+
// Create the constructor function
84+
JSValue ctor = JS_NewCFunction2(ctx, qjs_proxy_value_constructor, "QJS_PROXY_VALUE", 1, JS_CFUNC_constructor, 0);
85+
86+
// Set proto.constructor and ctor.prototype using QuickJS helper
87+
JS_SetConstructor(ctx, ctor, proto);
88+
89+
// Add the constructor to the global object
90+
if (JS_SetPropertyStr(ctx, global_obj, "QJS_PROXY_VALUE", ctor) < 0)
91+
{
92+
JS_FreeValue(ctx, global_obj);
93+
return -1;
94+
}
95+
96+
JS_FreeValue(ctx, global_obj);
97+
return 0;
98+
}
99+
100+
// Create a new QJS_PROXY_VALUE instance directly in C for better performance
101+
JSValue QJS_NewProxyValue(JSContext *ctx, int64_t proxyId)
102+
{
103+
// Get the QJS_PROXY_VALUE constructor from global object
104+
JSValue global_obj = JS_GetGlobalObject(ctx);
105+
JSValue ctor = JS_GetPropertyStr(ctx, global_obj, "QJS_PROXY_VALUE");
106+
JS_FreeValue(ctx, global_obj);
107+
108+
if (JS_IsException(ctor) || JS_IsUndefined(ctor))
109+
{
110+
JS_FreeValue(ctx, ctor);
111+
return JS_ThrowReferenceError(ctx, "QJS_PROXY_VALUE is not defined");
112+
}
113+
114+
// Create argument for the constructor (proxyId)
115+
JSValue arg = JS_NewInt64(ctx, proxyId);
116+
JSValue args[1] = {arg};
117+
118+
// Call the constructor with 'new'
119+
JSValue result = JS_CallConstructor(ctx, ctor, 1, args);
120+
121+
// Clean up
122+
JS_FreeValue(ctx, ctor);
123+
JS_FreeValue(ctx, arg);
124+
125+
return result;
126+
}

0 commit comments

Comments
 (0)