bnoordhuis
commented
3 hours ago Observation: if I take and release the mutex inside js_agent_sleep, the warnings go away.
```diff
diff --git a/quickjs.c b/quickjs.c
index 6467667..61eb29e 100644
--- a/quickjs.c
+++ b/quickjs.c
@@ -53574,7 +53574,7 @@ typedef struct JSAtomicsWaiter {
} JSAtomicsWaiter;
static js_once_t js_atomics_once = JS_ONCE_INIT;
-static js_mutex_t js_atomics_mutex;
+js_mutex_t js_atomics_mutex;
static struct list_head js_atomics_waiter_list =
LIST_HEAD_INIT(js_atomics_waiter_list);
diff --git a/run-test262.c b/run-test262.c
index b9928a9..5fd98fc 100644
--- a/run-test262.c
+++ b/run-test262.c
@@ -39,6 +39,8 @@
#include "list.h"
#include "quickjs-libc.h"
+extern js_mutex_t js_atomics_mutex;
+
/* enable test262 thread support to test SharedArrayBuffer and Atomics */
#define CONFIG_AGENT
@@ -651,6 +653,8 @@ static JSValue js_agent_sleep(JSContext *ctx, JSValue this_val,
uint32_t duration;
if (JS_ToUint32(ctx, &duration, argv[0]))
return JS_EXCEPTION;
+ js_mutex_lock(&js_atomics_mutex);
+ js_mutex_unlock(&js_atomics_mutex);
usleep(duration * 1000);
return JS_UNDEFINED;
}
```
@saghul thoughts on whether that's a good idea or a bad idea? It doesn't materially affect run-test262 running time; it's around 20 seconds both ways for all of test262/test/built-ins/Atomics
real 0m20,143s
user 0m7,219s
sys 0m0,171sAnd just Atomics.wait:
real 0m8,072s
user 0m2,609s
sys 0m0,140s(pretty big wall-clock vs. CPU time difference)
saghul
Tracking issue because I'm trying to add TSan support but I'm hitting a snag - it doesn't like how we implement JS atomics and that may or may not be a legitimate complaint:
:arrow_up: excerpt from running
build/run-test262 -c test262.conf -d test262/test/built-ins/Atomics/waitI'm undecided if the
as if synchronized via sleepwarning is a red herring or not but the observation that we mix atomic ops and mutexes is legit.CMakeLists.txt diff: