Interpreter: fix fiber's resumable property

spec/std/fiber_spec.cr

@@ -0,0 +1,21 @@
+require "spec"
+
+describe Fiber do
+  it "#resumable?" do
+    done = false
+    resumable = nil
+
+    fiber = spawn do
+      resumable = Fiber.current.resumable?
+      done = true
+    end
+
+    fiber.resumable?.should be_true
+
+    until done
+      Fiber.yield
+    end
+
+    resumable.should be_false
+  end
+end

src/compiler/crystal/interpreter/instructions.cr

@@ -1649,6 +1649,11 @@ require "./repl"
         pop_values: [current_context : Void*, new_context : Void*],
         code:       swapcontext(current_context, new_context),
       },
+      interpreter_fiber_resumable: {
+        pop_values: [context : Void*],
+        push:       true,
+        code:       fiber_resumable(context),
+      },
 
       {% if flag?(:bits64) %}
         interpreter_intrinsics_memcpy: {

src/compiler/crystal/interpreter/interpreter.cr

@@ -1154,18 +1154,25 @@ class Crystal::Repl::Interpreter
         nil
       end
     end
+    spawned_fiber.@context.resumable = 1
     spawned_fiber.as(Void*)
   end
 
   private def swapcontext(current_context : Void*, new_context : Void*)
     # current_fiber = current_context.as(Fiber*).value
     new_fiber = new_context.as(Fiber*).value
 
-    # We directly resume the next fiber.
-    # TODO: is this okay? We totally ignore the scheduler here!
+    # delegates the context switch to the interpreter's scheduler, so we update
+    # the current fiber reference, set the GC stack bottom, and so on (aka
+    # there's more to switching context than `Fiber.swapcontext`):
     new_fiber.resume
   end
 
+  private def fiber_resumable(context : Void*) : LibC::Long
+    fiber = context.as(Fiber*).value
+    fiber.@context.resumable
+  end
+
   private def pry(ip, instructions, stack_bottom, stack)
     offset = (ip - instructions.instructions.to_unsafe).to_i32
     node = instructions.nodes[offset]?

src/compiler/crystal/interpreter/primitives.cr

@@ -402,6 +402,9 @@ class Crystal::Repl::Compiler
     when "interpreter_fiber_swapcontext"
       accept_call_args(node)
       interpreter_fiber_swapcontext(node: node)
+    when "interpreter_fiber_resumable"
+      accept_call_args(node)
+      interpreter_fiber_resumable(node: node)
     when "interpreter_intrinsics_memcpy"
       accept_call_args(node)
       interpreter_intrinsics_memcpy(node: node)

src/crystal/interpreter.cr

@@ -19,5 +19,10 @@ module Crystal
     @[Primitive(:interpreter_spawn)]
     def self.spawn(fiber : Fiber, fiber_main : Void*) : Void*
     end
+
+    # Returns the resumable value from the interpreter's fiber.
+    @[Primitive(:interpreter_fiber_resumable)]
+    def self.fiber_resumable(context) : LibC::Long
+    end
   end
 end

src/crystal/scheduler.cr

@@ -125,14 +125,6 @@ class Crystal::Scheduler
     {% end %}
 
     current, @current = @current, fiber
-
-    {% if flag?(:interpreted) %}
-      # TODO: ideally we could set this in the interpreter if the
-      # @context had a pointer back to the fiber.
-      # I also wonder why this isn't done always like that instead of in asm.
-      current.@context.resumable = 1
-    {% end %}
-
     Fiber.swapcontext(pointerof(current.@context), pointerof(fiber.@context))
 
     {% if flag?(:preview_mt) %}

src/fiber/context.cr

@@ -7,10 +7,26 @@ class Fiber
   @[Extern]
   struct Context
     property stack_top : Void*
-    property resumable : LibC::Long
+
+    {% if flag?(:interpreted) %}
+      # In interpreted mode, the interpreted fibers will be backed by a real
+      # fiber run by the interpreter. The fiber context is thus fake.
+      #
+      # The `stack_top` property is actually a pointer to the real Fiber
+      # running in the interpreter.
+      #
+      # The `resumable` property is also delegated to the real fiber. Only the
+      # getter is defined (so we know the real state of the fiber); we don't
+      # declare a setter because only the interpreter can manipulate it (in the
+      # `makecontext` and `swapcontext` primitives).
+      def resumable : LibC::Long
+        Crystal::Interpreter.fiber_resumable(pointerof(@stack_top))
+      end
+    {% else %}
+      property resumable : LibC::Long = 0
+    {% end %}
 
     def initialize(@stack_top = Pointer(Void).null)
-      @resumable = 0
     end
   end
 

src/fiber/context/interpreted.cr

@@ -5,9 +5,9 @@ require "crystal/interpreter"
 class Fiber
   # :nodoc:
   def makecontext(stack_ptr, fiber_main) : Nil
-    # In interpreted mode the stack_top variable actually points to a fiber
+    # In interpreted mode the stack_top variable actually points to the actual
+    # fiber running on the interpreter
     @context.stack_top = Crystal::Interpreter.spawn(self, fiber_main.pointer)
-    @context.resumable = 1
   end
 
   # :nodoc: