Fiber context switch, Stack pools, Safe select

[ysbaddaden]

Rework of #6990 that introduces Fiber::Context to hold the stack pointer along with a resumable flag, used to determine whether the fiber is currently running (nice for MT), along with an alive flag on the fiber to know when it's dead and musn't be resumed anymore.

The scheduler now only resumes a resumable fiber, and will abort with a fatal error whenever trying to resume a running or dead fibers, that would lead to segfaults. It doesn't just ignore these fibers because they're proof of synchronization issues.

This change makes it obvious that select is buggy (see #3900 and #7348). I propose a full rewrite of Channel.select that spawns a fiber for each clause. It's probably much slower than the current implementation, but it's concurrency safe —thought we can probably speed things up by checking if any clause is ready, avoiding to spawn fibers until we must wait. I didn't benchmark, but I expect this implementation to be slower, since unless a clause is ready or there is an else branch, it now spawns N fibers just to cancel them eventually.

Notes:

• keeps Fiber::StackPool from Fiber context & Stack pool #6990 —maybe we don't need the mutex now;
• maybe these 3 commits could be 3 distinct pull requests.

[forksaber]

Hi @ysbaddaden,
I found an issue with this PR which is caused by not calling unwait immediately before resume/enqueue. The following example hangs but it shouldn't

ch1 = Channel(Int32).new(1)
ch2 = Channel(Int32).new
spawn do
  select
  when ch1.send(10)
  when ch2.send(20)
  end

  select
  when ch2.send(21)
  when ch1.send(11)
  end
end

select
when x = ch1.receive
  puts x
when y = ch2.receive
  puts y
end
puts ch2.receive

Expected output

10
21

[ysbaddaden]

@forksaber thanks for testing!

It sure hangs, but it's because ch2 is unbuffered (synchronous), while ch1 is unbuffered (asynchronous) an ch2 never gets a chance to actually send a value —it's always canceled—, so its empty, and the last ch2.receive will block. The flow goes like this:

ch1.send(10) # => success
ch1.send(11) # => waiting...
ch1.receive  # => 11
ch1.send(11) # => success
ch2.receive  # => waiting...

Once a clause is ready, we immediately cancel/unwait all other operations/fibers. Since we're single threaded, there is no race condition —with MT we'll need an atomic flag (easy) and be resilient to the execute operation failing (complex).

[forksaber]

I reported it only because the above code doesn't block with current version of crystal. I can assume there is slight behaviour change with your implementation. Whether that is considered a bug or not is debatable.

Anyways here's another test case:

ch1 = Channel(Int32).new(1)
ch2 = Channel(Int32).new
spawn do
  sleep 0
  ch1.send(10)
  ch2.send(21)
end

select
when x = ch1.receive
  puts x
when y = ch2.receive
  puts y
end
puts ch2.receive

Output

FATAL: tried to resume a dead fiber: #<Fiber:0x7fbb43997d80: 0>

Any thoughts on my approach (#7414 ) to solving this issue?

[ysbaddaden]

Oh, good catch: because ch1.send is buffered it enqueues the fiber of the ch1.receive select clause, but because ch2.send is unbuffered it immediately resumes the fiber of the ch2.receive clause, which will unwait other clauses, thus enqueue the first fiber a second time.