janet-lang / janet

A dynamic language and bytecode vm
https://janet-lang.org
MIT License
3.38k stars 217 forks source link

error: cannot marshal fiber with c stackframe #1326

Closed zevv closed 8 months ago

zevv commented 8 months ago

Reporting for a friend who's too lazy to report this himself.

The snippet below results in

instantiated (ev/call fun)
sleeping...
process exiting with code 0
instantiated (ev/go (short-fn (ev/do-thread (fun))))
error: cannot marshal fiber with c stackframe
  in ev/thread [src/core/ev.c] on line 2852
  in <anonymous> [disruptek.janet] on line 24, column 14

Removing the do makes this problem go away.

(defn spawn-sleep
  []
  (print "sleeping...")
  (let [process (os/spawn ["sleep" "3"] :p {})
        pid (process :pid)]
    (os/proc-close process)
    (printf "process exiting with code %d"
            (process :return-code))))

(defmacro test
  [form]
  ~(let [fib ,form]
     (printf "instantiated %q" ',form)
     (forever
       (match (fiber/status fib)
         :dead
         (break)
         status
         (ev/sleep 1)))))

(do
(def fun |(spawn-sleep))
(test (ev/call fun))
(test (ev/go |(ev/do-thread (fun))))
)
bakpakin commented 8 months ago

This is unfortunate behavior relating to closure capture. When you wrap the (test ...) code in the do form, and then create a local binding fun around |(spawn-sleep), you are accidentally capturing too much state with the ev/do-thread macro and that results in the above error.

I don't know if this is easy to fix, but I can look into improving closure capture to not capture the parent fiber.

Note that this also goes away if you use spawn-sleep directly, or move (def fun |(spawn-sleep)) out of the do block.

bakpakin commented 8 months ago

I'm going to mark this as fixed for now, but I still think there are some improvements to be made here. While the fixes above prevent this kind of error from happening in the case of this macro, we still could reduce variable capture from closures even more than we do already.