[quickjs-devel] Re: stackoverflow

  • From: Charlie Gordon <qemacs@xxxxxxxxxxx>
  • To: quickjs-devel@xxxxxxxxxxxxx
  • Date: Tue, 13 Aug 2019 12:04:42 +0200

Hi guys,

Are you calling JS_Eval() from a different thread than the one where the 
JSRuntime or the JSContext were instantiated?

QuickJS does not support multi-threading at the moment and is unlikely with the 
curent implementation to allow concurrent access to the same JSRuntime.

Yet calling JS_Eval and possibly other APIs from different threads 
sequentially, without concurrent access, as from workers, should be possible.

The problem is in the curent method used to detect stack overflow: we want to 
have a very efficient test for this and compare the current stack frame address 
with a reference value ctx->stack_top, set from an earlier observation of the 
said stack frame address, to determine if the exceeds ctx->stack_size. As noted 
by Alexander,  the test was broken in the first release because the current 
stack depth `ctx->stack_top - js_get_stack_pointer()` was assumed to be non 
negative and stored in a `size_t`.

Changing the type to `ptrdiff_t` is not a correct fix: a negative stack depth 
is indicative of something fishy, such as calling `js_check_stack_overflow()` 
from a stack frame that is not a descendant of the one where `ctx->stack_top` 
was set. Calling from a different thread is a good example.

There are multiple solutions, a combinaison of which should fix the issue:

* set `ctx->stack_top` in JS_Eval(), but this would prevent stack overflow 
detection in cases where javascript and C code are nested recursively.
* detecting calls from different threads in JS_Eval and other APIs, and set 
`ctx->stack_top` only in this case.
* make `stack_top` a _Thread_local variable instead of a member of  JSContext, 
and set it one way or another for every thread that invokes the 
compiler/interpreter

We should also prevent stack overflows in the compiler.

Both of these issues will be fixed in an upcoming release.

In the mean time, you can disable stack overflow checks in your applications by 
changing the `#if defined(EMSCRIPTEN)` to `#if 1` above the definition of 
`js_get_stack_pointer()`.

Best regards

Chqrlie.

On 13 Aug 2019, at 11:13, Alexander Rodin <rodin.alexander@xxxxxxxxx> wrote:

Hi Huib-Jan,

When I call JS_Eval from different places in my C code I often get a 
stackoverflow exception. This happens when __builtin_frame_address returns a 
higher address than it returned when the JSContext was created. Am I doing 
something wrong?


I’ve encountered this issue too. Please look at 
https://www.freelists.org/post/quickjs-devel/Stack-overflow-exception-caused-by-signed-to-unsigned-integer-conversion
 
<https://www.freelists.org/post/quickjs-devel/Stack-overflow-exception-caused-by-signed-to-unsigned-integer-conversion>
 for the patch.

Best regards,
Alexander

Other related posts: