[ruby-core:122922] [Ruby Feature#21039] Ractor.make_shareable breaks block semantics (seeing updated captured variables) of existing blocks

Issue #21039 has been updated by tenderlovemaking (Aaron Patterson).


After chatting a bit with @Eregon, I'd like to make a proposal about Ractor shareable procs.

I think it's going to be very difficult to port existing code to use Ractors if we can't have some way of creating shareable procs from non-literal blocks.  For example we wouldn't be able to use a Ractor based webserver with existing Sinatra applications.

With regard to local variables captured in environments, I think we should have the following rules:

1. Allow as many local writes as the user wants before the lambda captures the environment
2. Disallow any writes to the "now shared" environment after the lambda captures it
3. Disallow writes to the shared environment from inside the lambda

These rules should only apply to local variables declared outside the block.

Here are some examples to demonstrate the rules:

```ruby
## OK

foo = 123
Ractor.shareable_proc { foo }
p foo
```

```ruby
# NG: Raise an exception when creating a shareable proc
# The reason is because we're setting a local after the proc
# is created. This can cause possible race conditions / crashes

foo = 123
Ractor.shareable_proc { foo }
foo = Object.new # reassignment isn't allowed
```

```ruby
# NG: Raise an exception when creating a shareable proc
# The proc shouldn't be allowed to mutate a shared environment

foo = 123
Ractor.shareable_proc {
  foo += 1 # Not allowed because other env can't see mutation
}
```

```ruby
# Works, but value of `foo` may be unexpected.
# The second assignment should be ignored because the env is copied

foo = 123
Ractor.shareable_proc { foo }
eval("foo = Object.new")
```

```ruby
# Works, but value of `foo` outside the proc may be unexpected
# Proc only mutates its copied env

foo = 123
Ractor.shareable_proc {
  eval("foo += 1")
}
```

If we can enforce these rules, then I think it should be fine for `Ractor.shareable_proc` to take a block that _isn't_ a literal.  I also think this would allow a much larger number of existing proc objects to work safely with Ractors.

----------------------------------------
Feature #21039: Ractor.make_shareable breaks block semantics (seeing updated captured variables) of existing blocks
https://bugs.ruby-lang.org/issues/21039#change-114229

* Author: Eregon (Benoit Daloze)
* Status: Assigned
* Assignee: ko1 (Koichi Sasada)
----------------------------------------
```ruby
def make_counter
  count = 0
  nil.instance_exec do
    [-> { count }, -> { count += 1 }]
  end
end

get, increment = make_counter

reader = Thread.new {
  sleep 0.01
  loop do
    p get.call
    sleep 0.1
  end
}

writer = Thread.new {
  loop do
    increment.call
    sleep 0.1
  end
}

ractor_thread = Thread.new {
  sleep 1
  Ractor.make_shareable(get)
}

sleep 2
```

This prints:
```
1
2
3
4
5
6
7
8
9
10
10
10
10
10
10
10
10
10
10
10
```
But it should print 1..20, and indeed it does when commenting out the `Ractor.make_shareable(get)`.

This shows a given block/Proc instance is concurrently broken by `Ractor.make_shareable`, IOW Ractor is breaking fundamental Ruby semantics of blocks and their captured/outer variables or "environment".

It's expected that `Ractor.make_shareable` can `freeze` objects and that may cause some FrozenError, but here it's not a FrozenError, it's wrong/stale values being read.

I think what should happen instead is that `Ractor.make_shareable` should create a new Proc and mutate that.
However, if the Proc is inside some other object and not just directly the argument, that wouldn't work (like `Ractor.make_shareable([get])`).

So I think one fix would to be to only accept Procs for `Ractor.make_shareable(obj, copy: true)`.
FWIW that currently doesn't allow Procs, it gives `<internal:ractor>:828:in 'Ractor.make_shareable': allocator undefined for Proc (TypeError)`.
It makes sense to use `copy` here since `make_shareable` effectively takes a copy/snapshot of the Proc's environment.

I think the only other way, and I think it would be a far better way would be to not support making Procs shareable with `Ractor.make_shareable`.
Instead it could be some new method like `isolated { ... }` or `Proc.isolated { ... }` or `Proc.snapshot_outer_variables { ... }` or so, only accepting a literal block (to avoid mutating/breaking an existing block), and that would snapshot outer variables (or require no outer variables like Ractor.new's block, or maybe even do `Ractor.make_shareable(copy: true)` on outer variables) and possibly also set `self` since that's anyway needed.
That would make such blocks with different semantics explicit, which would fix the problem of breaking the intention of who wrote that block and whoever read that code, expecting normal Ruby block semantics, which includes seeing updated outer variables.
Related: #21033 https://bugs.ruby-lang.org/issues/18243#note-5

Extracted from https://bugs.ruby-lang.org/issues/21033#note-14



-- 
https://bugs.ruby-lang.org/
______________________________________________
 ruby-core mailing list -- [e-mail]
 To unsubscribe send an email to [e-mail]
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/