crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.33k stars 1.61k forks source link

Buffered Channel Regression #10977

Closed Blacksmoke16 closed 3 years ago

Blacksmoke16 commented 3 years ago

Using the code from https://crystal-lang.org/reference/guides/concurrency.html#buffered-channels on latest crystal currently outputs:

Before send 1
Before send 2
Before send 3
After send
1
2
3

https://play.crystal-lang.org/#/r/bliz

This seems to be a regression from 0.31.0 (when MT was first added) as the same code from 0.30.1 produces the correct output:

Before send 1
Before send 2
Before send 3
1
2
After send
3

https://play.crystal-lang.org/#/r/blix

straight-shoota commented 3 years ago

The regression was introduced in #8112 and thus already released in 0.31.0.

asterite commented 3 years ago

I think the guide should be very explicit about users not relying on the order of execution of threads/fibers.

lbguilherme commented 3 years ago

Actually, the documentation says we can rely on the execution order of a channel. Take this example:

channel = Channel(Int32).new

spawn do
  4.times do
    puts "ping"
    channel.send(0)
  end
end

4.times do
  channel.receive
  puts "pong"
end

Here the channel is unbuffered, this is what the documentation says:

Sends a value to the channel. If the channel has spare capacity, then the method returns immediately. Otherwise, this method blocks the calling fiber until another fiber calls #receive on the channel.

Thus the expected outcome should be:

ping
pong
ping
pong
ping
pong
ping
pong

But instead, it is:

ping
ping
pong
pong
ping
ping
pong
pong

The first send didn't block the fiber. The same can be extended to a buffered channel, it looks like the real capacity is always +1 from the requested capacity.

straight-shoota commented 3 years ago

@asterite That's true, of course. But when it comes to synchronization between channels, some expectations about the order of execution are totally legit.

However, I found the current behaviour is actually correct. The first send is not added to the buffer queue because there's already a receiver waiting (the send fiber only starts running after the first receive call blocks the main fiber). So the queue starts filling at the second send call and the third one still fits.

So we really just need to update the example to represent the actual behaviour.

lbguilherme commented 3 years ago

Thanks @straight-shoota.

Looking at the optics that the receiving fiber has already called receive and is blocked, it makes sense for send to unblock the receiving fiber without blocking the sending one and without consuming queue space. I always assumed in-process-of-being-received data occupied space in the queue, but that's not true.

For buffered channel it makes sense as is, just a matter of improving the example (and maybe documentation?). For the unbuffered one, we can decide if we want to switch to a receiving fiber as part of sending or if the current behavior is fine. I now think it is.

straight-shoota commented 3 years ago

Resolved by crystal-lang/crystal-book#537