client: make quit() actually quit, misc fixes

* unwind the QJS stack with an uncatchable exception when quit is called
* clean up JS references in JSRuntime free even when the Nim
  counterparts are still alive
* simplify some tests

13 files changed, 131 insertions, 138 deletions

diff --git a/lib/quickjs/quickjs.h b/lib/quickjs/quickjs.h
index 7b6edd1f..89fbca34 100644
--- a/lib/quickjs/quickjs.h
+++ b/lib/quickjs/quickjs.h
@@ -636,6 +636,7 @@ static inline JS_BOOL JS_IsObject(JSValueConst v)
 JSValue JS_Throw(JSContext *ctx, JSValue obj);
 JSValue JS_GetException(JSContext *ctx);
 JS_BOOL JS_IsError(JSContext *ctx, JSValueConst val);
+void JS_SetUncatchableError(JSContext *ctx , JSValueConst val, JS_BOOL flag);
 void JS_ResetUncatchableError(JSContext *ctx);
 JSValue JS_NewError(JSContext *ctx);
 JSValue __js_printf_like(2, 3) JS_ThrowSyntaxError(JSContext *ctx, const char *fmt, ...);
diff --git a/src/bindings/quickjs.nim b/src/bindings/quickjs.nim
index 7a3eee54..06d8087f 100644
--- a/src/bindings/quickjs.nim
+++ b/src/bindings/quickjs.nim
@@ -526,6 +526,8 @@ proc JS_IsArray*(ctx: JSContext; v: JSValue): cint
 proc JS_Throw*(ctx: JSContext; obj: JSValue): JSValue
 proc JS_GetException*(ctx: JSContext): JSValue
 proc JS_IsError*(ctx: JSContext; v: JSValue): JS_BOOL
+proc JS_SetUncatchableError*(ctx: JSContext; val: JSValue; flag: JS_BOOL)
+proc JS_ResetUncatchableError*(ctx: JSContext)
 proc JS_NewError*(ctx: JSContext): JSValue
 proc JS_ThrowSyntaxError*(ctx: JSContext; fmt: cstring): JSValue {.varargs,
   discardable.}
diff --git a/src/js/domexception.nim b/src/js/domexception.nim
index f916fb59..39537b0a 100644
--- a/src/js/domexception.nim
+++ b/src/js/domexception.nim
@@ -38,11 +38,7 @@ type
 jsDestructor(DOMException)
 
 proc newDOMException*(message = ""; name = "Error"): DOMException {.jsctor.} =
-  return DOMException(
-    e: JS_DOM_EXCEPTION,
-    name: name,
-    message: message
-  )
+  return DOMException(e: jeDOMException, name: name, message: message)
 
 template errDOMException*(message, name: string): untyped =
   err(newDOMException(message, name))
@@ -54,4 +50,4 @@ func code(this: DOMException): uint16 {.jsfget.} =
   return NamesTable.getOrDefault(this.name, 0u16)
 
 proc addDOMExceptionModule*(ctx: JSContext) =
-  ctx.registerType(DOMException, JS_CLASS_ERROR, errid = opt(JS_DOM_EXCEPTION))
+  ctx.registerType(DOMException, JS_CLASS_ERROR, errid = opt(jeDOMException))
diff --git a/src/js/error.nim b/src/js/error.nim
index 860f4f80..b4101830 100644
--- a/src/js/error.nim
+++ b/src/js/error.nim
@@ -7,77 +7,53 @@ type
 
   JSErrorEnum* = enum
     # QuickJS internal errors
-    JS_EVAL_ERROR0 = "EvalError"
-    JS_RANGE_ERROR0 = "RangeError"
-    JS_REFERENCE_ERROR0 = "ReferenceError"
-    JS_SYNTAX_ERROR0 = "SyntaxError"
-    JS_TYPE_ERROR0 = "TypeError"
-    JS_URI_ERROR0 = "URIError"
-    JS_INTERNAL_ERROR0 = "InternalError"
-    JS_AGGREGATE_ERROR0 = "AggregateError"
+    jeEvalError = "EvalError"
+    jeRangeError = "RangeError"
+    jeReferenceError = "ReferenceError"
+    jeSyntaxError = "SyntaxError"
+    jeTypeError = "TypeError"
+    jeURIError = "URIError"
+    jeInternalError = "InternalError"
+    jeAggregateError = "AggregateError"
     # Chawan errors
-    JS_DOM_EXCEPTION = "DOMException"
+    jeDOMException = "DOMException"
 
   JSResult*[T] = Result[T, JSError]
 
 const QuickJSErrors* = [
-  JS_EVAL_ERROR0,
-  JS_RANGE_ERROR0,
-  JS_REFERENCE_ERROR0,
-  JS_SYNTAX_ERROR0,
-  JS_TYPE_ERROR0,
-  JS_URI_ERROR0,
-  JS_INTERNAL_ERROR0,
-  JS_AGGREGATE_ERROR0
+  jeEvalError,
+  jeRangeError,
+  jeReferenceError,
+  jeSyntaxError,
+  jeTypeError,
+  jeURIError,
+  jeInternalError,
+  jeAggregateError
 ]
 
 proc newEvalError*(message: string): JSError =
-  return JSError(
-    e: JS_EVAL_ERROR0,
-    message: message
-  )
+  return JSError(e: jeEvalError, message: message)
 
 proc newRangeError*(message: string): JSError =
-  return JSError(
-    e: JS_RANGE_ERROR0,
-    message: message
-  )
+  return JSError(e: jeRangeError, message: message)
 
 proc newReferenceError*(message: string): JSError =
-  return JSError(
-    e: JS_REFERENCE_ERROR0,
-    message: message
-  )
+  return JSError(e: jeReferenceError, message: message)
 
 proc newSyntaxError*(message: string): JSError =
-  return JSError(
-    e: JS_SYNTAX_ERROR0,
-    message: message
-  )
+  return JSError(e: jeSyntaxError, message: message)
 
 proc newTypeError*(message: string): JSError =
-  return JSError(
-    e: JS_TYPE_ERROR0,
-    message: message
-  )
+  return JSError(e: jeTypeError, message: message)
 
 proc newURIError*(message: string): JSError =
-  return JSError(
-    e: JS_URI_ERROR0,
-    message: message
-  )
+  return JSError(e: jeURIError, message: message)
 
 proc newInternalError*(message: string): JSError =
-  return JSError(
-    e: JS_INTERNAL_ERROR0,
-    message: message
-  )
+  return JSError(e: jeInternalError, message: message)
 
 proc newAggregateError*(message: string): JSError =
-  return JSError(
-    e: JS_AGGREGATE_ERROR0,
-    message: message
-  )
+  return JSError(e: jeAggregateError, message: message)
 
 template errTypeError*(message: string): untyped =
   err(newTypeError(message))
diff --git a/src/js/javascript.nim b/src/js/javascript.nim
index 2266a3c7..44c7ef06 100644
--- a/src/js/javascript.nim
+++ b/src/js/javascript.nim
@@ -180,6 +180,15 @@ proc free*(ctx: JSContext) =
 proc free*(rt: JSRuntime) =
   let opaque = rt.getOpaque()
   GC_unref(opaque)
+  var ps: seq[pointer] = @[]
+  for p in opaque.plist.values:
+    ps.add(p)
+  opaque.plist.clear()
+  for p in ps:
+    #TODO maybe finalize?
+    let val = JS_MKPTR(JS_TAG_OBJECT, p)
+    JS_SetOpaque(val, nil)
+    JS_FreeValueRT(rt, val)
   JS_FreeRuntime(rt)
   runtimes.del(runtimes.find(rt))
 
diff --git a/src/js/tojs.nim b/src/js/tojs.nim
index 7831a6a9..ea6f35f2 100644
--- a/src/js/tojs.nim
+++ b/src/js/tojs.nim
@@ -299,7 +299,7 @@ proc toJSNew*(ctx: JSContext; obj: ref object; ctor: JSValue): JSValue =
     GC_ref(obj)
   return val
 
-proc toJSNew[T, E](ctx: JSContext; opt: Result[T, E], ctor: JSValue): JSValue =
+proc toJSNew[T, E](ctx: JSContext; opt: Result[T, E]; ctor: JSValue): JSValue =
   if opt.isSome:
     when not (T is void):
       return toJSNew(ctx, opt.get, ctor)
diff --git a/src/local/client.nim b/src/local/client.nim
index 359d143c..c08f88f6 100644
--- a/src/local/client.nim
+++ b/src/local/client.nim
@@ -37,6 +37,7 @@ import js/javascript
 import js/jstypes
 import js/jsutils
 import js/module
+import js/opaque
 import js/timeout
 import js/tojs
 import loader/headers
@@ -59,7 +60,6 @@ import chagashi/charset
 type
   Client* = ref object
     alive: bool
-    dead: bool
     config {.jsget.}: Config
     consoleWrapper: ConsoleWrapper
     fdmap: Table[int, Container]
@@ -70,6 +70,7 @@ type
     timeouts: TimeoutState
     pressed: tuple[col: int; row: int]
     exitCode: int
+    inEval: bool
 
   ConsoleWrapper = object
     console: Console
@@ -114,15 +115,39 @@ proc interruptHandler(rt: JSRuntime; opaque: pointer): cint {.cdecl.} =
 proc runJSJobs(client: Client) =
   client.jsrt.runJSJobs(client.console.err)
 
+proc cleanup(client: Client) =
+  if client.alive:
+    client.alive = false
+    client.pager.quit()
+    for val in client.config.cmd.map.values:
+      JS_FreeValue(client.jsctx, val)
+    for fn in client.config.jsvfns:
+      JS_FreeValue(client.jsctx, fn)
+    assert not client.inEval
+    client.jsctx.free()
+    client.jsrt.free()
+
+proc quit(client: Client; code = 0) =
+  client.cleanup()
+  quit(code)
+
 proc evalJS(client: Client; src, filename: string; module = false): JSValue =
   client.pager.term.unblockStdin()
   let flags = if module:
     JS_EVAL_TYPE_MODULE
   else:
     JS_EVAL_TYPE_GLOBAL
+  let wasInEval = client.inEval
+  client.inEval = true
   result = client.jsctx.eval(src, filename, flags)
-  client.runJSJobs()
+  client.inEval = false
   client.pager.term.restoreStdin()
+  if client.exitCode != -1:
+    # if we are in a nested eval, then just wait until we are not.
+    if not wasInEval:
+      client.quit(client.exitCode)
+  else:
+    client.runJSJobs()
 
 proc evalJSFree(client: Client; src, filename: string) =
   JS_FreeValue(client.jsctx, client.evalJS(src, filename))
@@ -150,32 +175,13 @@ proc suspend(client: Client) {.jsfunc.} =
   discard kill(0, cint(SIGTSTP))
   client.pager.term.restart()
 
-proc jsQuit(client: Client; code = 0) {.jsfunc: "quit".} =
-  client.exitCode = code
-  client.alive = false
-
-proc quit(client: Client; code = 0) =
-  if not client.dead:
-    # dead is set to true when quit is called; it indicates that the
-    # client has been destroyed.
-    # alive is set to false when jsQuit is called; it is a request to
-    # destroy the client.
-    client.dead = true
-    client.alive = false
-    client.pager.quit()
-    for val in client.config.cmd.map.values:
-      JS_FreeValue(client.jsctx, val)
-    for fn in client.config.jsvfns:
-      JS_FreeValue(client.jsctx, fn)
-    let ctx = client.jsctx
-    let rt = client.jsrt
-    # Force the runtime to collect all memory, so QJS can check for
-    # leaks.
-    client[].reset()
-    GC_fullCollect()
-    ctx.free()
-    rt.free()
-  exitnow(code)
+proc jsQuit(client: Client; code: uint32 = 0): JSValue {.jsfunc: "quit".} =
+  client.exitCode = int(code)
+  let ctx = client.jsctx
+  let ctor = ctx.getOpaque().errCtorRefs[jeInternalError]
+  let err = JS_CallConstructor(ctx, ctor, 1, JS_UNDEFINED.toJSValueArray())
+  JS_SetUncatchableError(ctx, err, true);
+  return JS_Throw(ctx, err)
 
 proc feedNext(client: Client) {.jsfunc.} =
   client.feednext = true
@@ -193,6 +199,7 @@ proc evalActionJS(client: Client; action: string): JSValue =
     return JS_DupValue(client.jsctx, p[])
   return client.evalJS(action, "<command>")
 
+# Warning: this is not re-entrant.
 proc evalAction(client: Client; action: string; arg0: int32): EmptyPromise =
   var ret = client.evalActionJS(action)
   let ctx = client.jsctx
@@ -209,6 +216,9 @@ proc evalAction(client: Client; action: string; arg0: int32): EmptyPromise =
       let ret2 = JS_Call(ctx, ret, JS_UNDEFINED, 0, nil)
       JS_FreeValue(ctx, ret)
       ret = ret2
+    if client.exitCode != -1:
+      assert not client.inEval
+      client.quit(client.exitCode)
   if JS_IsException(ret):
     client.jsctx.writeException(client.console.err)
   if JS_IsObject(ret):
@@ -553,7 +563,7 @@ proc inputLoop(client: Client) =
   let selector = client.selector
   selector.registerHandle(int(client.pager.term.istream.fd), {Read}, 0)
   let sigwinch = selector.registerSignal(int(SIGWINCH), 0)
-  while client.alive:
+  while true:
     let events = client.selector.select(-1)
     for event in events:
       if Read in event.events:
@@ -605,7 +615,7 @@ func hasSelectFds(client: Client): bool =
     client.pager.procmap.len > 0
 
 proc headlessLoop(client: Client) =
-  while client.alive and client.hasSelectFds():
+  while client.hasSelectFds():
     let events = client.selector.select(-1)
     for event in events:
       if Read in event.events:
@@ -741,8 +751,7 @@ proc launchClient*(client: Client; pages: seq[string];
   # better associate it with jsctx
   client.timeouts = newTimeoutState(client.selector, client.jsctx,
     client.console.err, proc(src, file: string) = client.evalJSFree(src, file))
-  client.alive = true
-  addExitProc((proc() = client.quit()))
+  addExitProc((proc() = client.cleanup()))
   if client.config.start.startup_script != "":
     let s = if fileExists(client.config.start.startup_script):
       readFile(client.config.start.startup_script)
@@ -765,7 +774,6 @@ proc launchClient*(client: Client; pages: seq[string];
     client.dumpBuffers()
   if client.config.start.headless:
     client.headlessLoop()
-  client.quit()
 
 proc nimGCStats(client: Client): string {.jsfunc.} =
   return GC_getStatistics()
@@ -836,6 +844,7 @@ proc newClient*(config: Config; forkserver: ForkServer; jsctx: JSContext;
     jsrt: jsrt,
     jsctx: jsctx,
     pager: pager,
+    exitCode: -1,
     alive: true
   )
   jsrt.setInterruptHandler(interruptHandler, cast[pointer](client))
diff --git a/src/local/pager.nim b/src/local/pager.nim
index 875f0df4..1c1f0530 100644
--- a/src/local/pager.nim
+++ b/src/local/pager.nim
@@ -282,7 +282,7 @@ proc dumpAlerts*(pager: Pager) =
   for msg in pager.alerts:
     stderr.write("cha: " & msg & '\n')
 
-proc quit*(pager: Pager; code = 0) =
+proc quit*(pager: Pager) =
   pager.term.quit()
   pager.dumpAlerts()
 
diff --git a/src/main.nim b/src/main.nim
index 338e9518..f2ca217b 100644
--- a/src/main.nim
+++ b/src/main.nim
@@ -62,6 +62,7 @@ type ParamParseContext = object
   visual: bool
   opts: seq[string]
   stylesheet: string
+  pages: seq[string]
 
 proc getnext(ctx: var ParamParseContext): string =
   inc ctx.i
@@ -110,16 +111,12 @@ proc parseRun(ctx: var ParamParseContext) =
   ctx.opts.add("start.headless = true")
   ctx.dump = true
 
-proc main() =
-  putEnv("CHA_LIBEXEC_DIR", ChaPath"${%CHA_LIBEXEC_DIR}".unquoteGet())
-  let forkserver = newForkServer()
-  var ctx = ParamParseContext(params: commandLineParams(), i: 0)
+proc parse(ctx: var ParamParseContext) =
   var escapeAll = false
-  var pages: seq[string] = @[]
   while ctx.i < ctx.params.len:
     let param = ctx.params[ctx.i]
     if escapeAll: # after --
-      pages.add(param)
+      ctx.pages.add(param)
       inc ctx.i
       continue
     if param.len == 0:
@@ -170,8 +167,14 @@ proc main() =
         of "--": escapeAll = true
         else: help(1)
     else:
-      pages.add(param)
+      ctx.pages.add(param)
     inc ctx.i
+
+proc main() =
+  putEnv("CHA_LIBEXEC_DIR", ChaPath"${%CHA_LIBEXEC_DIR}".unquoteGet())
+  let forkserver = newForkServer()
+  var ctx = ParamParseContext(params: commandLineParams(), i: 0)
+  ctx.parse()
   let jsrt = newJSRuntime()
   let jsctx = jsrt.newJSContext()
   var warnings = newSeq[string]()
@@ -193,14 +196,14 @@ proc main() =
       stderr.write("Error parsing commands: " & res.error)
       quit(1)
   set_cjk_ambiguous(config.display.double_width_ambiguous)
-  if pages.len == 0 and stdin.isatty():
+  if ctx.pages.len == 0 and stdin.isatty():
     if ctx.visual:
-      pages.add(config.start.visual_home)
+      ctx.pages.add(config.start.visual_home)
     elif (let httpHome = getEnv("HTTP_HOME"); httpHome != ""):
-      pages.add(httpHome)
+      ctx.pages.add(httpHome)
     elif (let wwwHome = getEnv("WWW_HOME"); wwwHome != ""):
-      pages.add(wwwHome)
-  if pages.len == 0 and not config.start.headless:
+      ctx.pages.add(wwwHome)
+  if ctx.pages.len == 0 and not config.start.headless:
     if stdin.isatty():
       help(1)
   # make sure tmpdir actually exists; if we do this later, then forkserver may
@@ -209,7 +212,7 @@ proc main() =
   forkserver.loadForkServerConfig(config)
   let client = newClient(config, forkserver, jsctx, warnings)
   try:
-    client.launchClient(pages, ctx.contentType, ctx.charset, ctx.dump)
+    client.launchClient(ctx.pages, ctx.contentType, ctx.charset, ctx.dump)
   except CatchableError:
     client.flushConsole()
     raise
diff --git a/test/js/htmlcollection.html b/test/js/htmlcollection.html
index 6231237a..2b5b4983 100644
--- a/test/js/htmlcollection.html
+++ b/test/js/htmlcollection.html
@@ -1,22 +1,21 @@
 <!doctype html>
 <title>HTMLCollection test</title>
 <div class="abc">Fail</div>
+<style>style 0</style>
 <style>style 1</style>
 <style>style 2</style>
-<style>style 3</style>
+<script src=asserts.js></script>
 <script>
-(function() {
-	const abc = document.getElementsByClassName("abc");
-	if (abc.length !== 1) return;
-	abc[0].className = "defg";
-	if (abc.length !== 0) return;
-	const styles = document.getElementsByTagName("style");
-	if (styles.length !== 3) return;
-	if (styles[0].textContent !== "style 1") return;
-	if (styles[1].textContent !== "style 2") return;
-	if (styles[2].textContent !== "style 3") return;
-	for (const style of styles) style.remove();
-	let defg = document.getElementsByClassName("defg");
-	defg[0].textContent = "Success";
-})()
+const abc = document.getElementsByClassName("abc");
+assert_equals(abc.length, 1);
+abc[0].className = "defg";
+assert_equals(abc.length, 0);
+const styles = document.getElementsByTagName("style");
+assert_equals(styles.length, 3);
+for (let i = 0; i < styles.length; ++i)
+	assert_equals(styles[i].textContent, "style " + i);
+for (const style of styles)
+	style.remove();
+const defg = document.getElementsByClassName("defg");
+defg[0].textContent = "Success";
 </script>
diff --git a/test/js/outerhtml.html b/test/js/outerhtml.html
index 5c30bc52..8e4bbd2c 100644
--- a/test/js/outerhtml.html
+++ b/test/js/outerhtml.html
@@ -1,9 +1,9 @@
 <!doctype html>
 <title>innerHTML test</title>
 <div id="test">Fail</div>
+<script src=asserts.js></script>
 <script>
 const div = document.getElementById("test");
-if (div.outerHTML != '<div id="test">Fail</div>')
-	throw new TypeError("invalid outerHTML " + div.outerHTML);
+assert_equals(div.outerHTML, '<div id="test">Fail</div>');
 div.textContent = "Success";
 </script>
diff --git a/test/js/reflect.html b/test/js/reflect.html
index 45debe39..9b2ed4cb 100644
--- a/test/js/reflect.html
+++ b/test/js/reflect.html
@@ -1,18 +1,17 @@
 <!doctype html>
 <div id=abc>Fail</div>
 <a class=claz target="abcd">test test</a>
+<script src=asserts.js></script>
 <script>
-(function() {
-	let x = document.getElementById("abc")
-	let a = document.getElementsByTagName("a")[0];
-	if (a.target != "abcd") return;
-	a.target = "defg";
-	if (a.target != "defg") return;
-	if (a.relList != "") return;
-	a.relList = "...";
-	if (a.relList != "...") return;
-	if (a.className != "claz") return;
-	x.textContent = "Success";
-	a.remove(); /* ignore target... */
-})()
+const x = document.getElementById("abc")
+const a = document.getElementsByTagName("a")[0];
+assert_equals(a.target, "abcd");
+a.target = "defg";
+assert_equals(a.target, "defg");
+assert_equals(a.relList + "", "");
+a.relList = "...";
+assert_equals(a.relList + "", "...");
+assert_equals(a.className, "claz");
+x.textContent = "Success";
+a.remove(); /* ignore target... */
 </script>
diff --git a/test/js/text.html b/test/js/text.html
index 7d7bcfca..8a511868 100644
--- a/test/js/text.html
+++ b/test/js/text.html
@@ -1,8 +1,7 @@
 <!doctype html>
-<div id="succ">Fail</div>
+<div id=x>Fail</div>
+<script src=asserts.js></script>
 <script>
-(function() {
-	if (new Text("data").ownerDocument != document) return;
-	document.getElementById("succ").textContent = "Success"
-})()
+assert_equals(new Text("data").ownerDocument, document);
+document.getElementById("x").textContent = "Success";
 </script>