[ruby-core:111789] [Ruby master Feature#19333] Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete value in order to avoid memory leaks.
11 Jan
2023
11 Jan
'23
10:21 p.m.
Issue #19333 has been reported by ioquatix (Samuel Williams).
----------------------------------------
Feature #19333: Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete
value in order to avoid memory leaks.
https://bugs.ruby-lang.org/issues/19333
* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As it stands, Fiber Locals, Thread Locals and Fiber Storage have no way of deleting
key-value associations.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
end
```
Because of this, dynamically generated associations can leak over time. This is worse for
things like Threads that might be pooled (or maybe an argument against user-space
pooling).
In any case, having a way to delete those associations would allow application code to at
least delete the associations when they no longer make sense.
I propose that assigning `nil` to "locals" or "storage" should
effectively delete them.
e.g.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
Thread.current[name] = nil # delete association
end
```
A more invasive alternative would be to define new interfaces like `Thread::Local`,
`Fiber::Local` and `Fiber::Storage::Local` (or something) which correctly clean up on GC.
--
https://bugs.ruby-lang.org/
16 Jan
16 Jan
9:53 a.m.
New subject: [ruby-core:111830] [Ruby master Feature#19333] Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete value in order to avoid memory leaks.
Issue #19333 has been updated by ioquatix (Samuel Williams).
While the proposed idea is a good move in the right direction, it doesn't handle cases
where several (a large number) threads all have the SAME thread local, and that thread
local goes out of scope, e.g.
```
name = ":local-#{object_id}
100.times do
Thread.new do
thing = ... something that consumes memory ...
Thread.current.thread_variable_set(name, thing)
# Thread lives for a long time
while job = jobs.pop
job.call
end
end
end
```
Even when `name` is no longer useful for any purpose, all those threads sill still have
that thread local set. There is no global way to say "For all threads that have a
thread local set for name, please clear it".
In order to do that, we'd need to introduce a `Thread::Local` object which uniquely
identifies an association in 0 or more threads, and when that `Thread::Local` instance is
GCed, all threads which have it set, have it cleared. This ensures that threads don't
leak memory. Consider a work pool that allocates a new `Thread::Local `every now and
then... eventually threads will have lots of locals which are potentially no longer
useful.
That being said, the other option is just not to encourage people to have long running
threads that accumulate cruft over time.
----------------------------------------
Feature #19333: Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete
value in order to avoid memory leaks.
https://bugs.ruby-lang.org/issues/19333#change-101239
* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As it stands, Fiber Locals, Thread Locals and Fiber Storage have no way of deleting
key-value associations.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
end
```
Because of this, dynamically generated associations can leak over time. This is worse for
things like Threads that might be pooled (or maybe an argument against user-space
pooling).
In any case, having a way to delete those associations would allow application code to at
least delete the associations when they no longer make sense.
I propose that assigning `nil` to "locals" or "storage" should
effectively delete them.
e.g.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
Thread.current[name] = nil # delete association
end
```
A more invasive alternative would be to define new interfaces like `Thread::Local`,
`Fiber::Local` and `Fiber::Storage::Local` (or something) which correctly clean up on GC.
--
https://bugs.ruby-lang.org/
11:53 a.m.
New subject: [ruby-core:111831] [Ruby master Feature#19333] Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete value in order to avoid memory leaks.
Issue #19333 has been updated by Eregon (Benoit Daloze).
The current solution/workaround to both of these memory leaks is to use
concurrent-ruby's
[ThreadLocalVar](https://ruby-concurrency.github.io/concurrent-ruby/master/Concurrent/ThreadLocalVar.html),
which is the reason that exists.
The idea to remove on assigning `nil` looks elegant at first sight but has multiple
problems:
* The user might use `nil` as a "valid value" and with this change it will be
more expensive to remove the fiber local than to just assign, also more expensive if later
that fiber local is set again to non-nil (delete+insert vs set+set).
* This is problematic when implementing fiber locals with Shapes, which TruffleRuby
currently does, because it causes a lot more Shape polymorphism if fiber locals can be
removed, so makes fiber locals slower.
* It doesn't solve the leak itself, it would need a finalizer per fiber local, which
is pretty heavy for this feature AND we can't put a finlizer on the Symbol since the
Symbol is never collected. So this seems unusable for fiber/thread locals, it would need
an extra wrapping object for every usage, or explicitly clearing the fiber local in a
begin/ensure/end pattern.
For those reasons I'm rather against this.
I think it's a rare enough problem that it's OK to rely on concurrent-ruby for
this feature.
Many fiber/thread local usages are fine because they don't use a random part in the
Symbol and so it's a small number of them.
It is a problem when there are many fiber/thread locals, IMO those should
concurrent-ruby's ThreadLocalVar/FiberLocalVar.
That being said, the other option is just not to
encourage people to have long running threads that accumulate cruft over time.
That's a hard sell, at least the not having long running threads part. On all current
Rubies the time to create a thread is significant and so reusing threads e.g. via a thread
pool is a necessity for performance (e.g., in threaded web servers like Puma). (maybe
CRuby can do M-N threading in the future, but this is not possible on JVM AFAIK because
e.g. ReentrantLock relies on java.lang.Thread identity and not RubyThread/RubyFiber
identity)
----------------------------------------
Feature #19333: Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete
value in order to avoid memory leaks.
https://bugs.ruby-lang.org/issues/19333#change-101240
* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As it stands, Fiber Locals, Thread Locals and Fiber Storage have no way of deleting
key-value associations.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
end
```
Because of this, dynamically generated associations can leak over time. This is worse for
things like Threads that might be pooled (or maybe an argument against user-space
pooling).
In any case, having a way to delete those associations would allow application code to at
least delete the associations when they no longer make sense.
I propose that assigning `nil` to "locals" or "storage" should
effectively delete them.
e.g.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
Thread.current[name] = nil # delete association
end
```
A more invasive alternative would be to define new interfaces like `Thread::Local`,
`Fiber::Local` and `Fiber::Storage::Local` (or something) which correctly clean up on GC.
--
https://bugs.ruby-lang.org/
17 Jan
17 Jan
6:40 a.m.
New subject: [ruby-core:111848] [Ruby master Feature#19333] Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete value in order to avoid memory leaks.
Issue #19333 has been updated by ioquatix (Samuel Williams).
The user might use nil as a "valid value"
Have you seen any examples of this?
----------------------------------------
Feature #19333: Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete
value in order to avoid memory leaks.
https://bugs.ruby-lang.org/issues/19333#change-101259
* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As it stands, Fiber Locals, Thread Locals and Fiber Storage have no way of deleting
key-value associations.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
end
```
Because of this, dynamically generated associations can leak over time. This is worse for
things like Threads that might be pooled (or maybe an argument against user-space
pooling).
In any case, having a way to delete those associations would allow application code to at
least delete the associations when they no longer make sense.
I propose that assigning `nil` to "locals" or "storage" should
effectively delete them.
e.g.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
Thread.current[name] = nil # delete association
end
```
A more invasive alternative would be to define new interfaces like `Thread::Local`,
`Fiber::Local` and `Fiber::Storage::Local` (or something) which correctly clean up on GC.
--
https://bugs.ruby-lang.org/
11:41 a.m.
New subject: [ruby-core:111850] [Ruby master Feature#19333] Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete value in order to avoid memory leaks.
Issue #19333 has been updated by Eregon (Benoit Daloze).
There are some, yes, see
https://github.com/search?q=%22Thread.current%5B%22+%22%5D+%3D+nil%22+langu…
Some are about cleaning after usage in `ensure`, but not all.
And even then it's not clear if it's never used again after.
For example I saw a `= nil` to reset a connection.
----------------------------------------
Feature #19333: Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete
value in order to avoid memory leaks.
https://bugs.ruby-lang.org/issues/19333#change-101262
* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As it stands, Fiber Locals, Thread Locals and Fiber Storage have no way of deleting
key-value associations.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
end
```
Because of this, dynamically generated associations can leak over time. This is worse for
things like Threads that might be pooled (or maybe an argument against user-space
pooling).
In any case, having a way to delete those associations would allow application code to at
least delete the associations when they no longer make sense.
I propose that assigning `nil` to "locals" or "storage" should
effectively delete them.
e.g.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
Thread.current[name] = nil # delete association
end
```
A more invasive alternative would be to define new interfaces like `Thread::Local`,
`Fiber::Local` and `Fiber::Storage::Local` (or something) which correctly clean up on GC.
--
https://bugs.ruby-lang.org/
10:13 p.m.
New subject: [ruby-core:111865] [Ruby master Feature#19333] Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete value in order to avoid memory leaks.
Issue #19333 has been updated by ioquatix (Samuel Williams).
Thanks for the code search link.
Regarding the usage, wouldn't it only matter if the variable was checked with
`thread_variable?` or equivalent? Because `thread_variable_get(:x)` is `nil` regardless of
whether it's set or not.
Assignment to `nil`, e.g. `thread_variable_set(:x) = nil`, and the subsequent deletion of
that variable, only has a side effect iif later behaviour is controlled by
`thread_variable?(:x)` AND that behaviour would be different in the presence of a `nil`
value.
----------------------------------------
Feature #19333: Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete
value in order to avoid memory leaks.
https://bugs.ruby-lang.org/issues/19333#change-101279
* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As it stands, Fiber Locals, Thread Locals and Fiber Storage have no way of deleting
key-value associations.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
end
```
Because of this, dynamically generated associations can leak over time. This is worse for
things like Threads that might be pooled (or maybe an argument against user-space
pooling).
In any case, having a way to delete those associations would allow application code to at
least delete the associations when they no longer make sense.
I propose that assigning `nil` to "locals" or "storage" should
effectively delete them.
e.g.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
Thread.current[name] = nil # delete association
end
```
A more invasive alternative would be to define new interfaces like `Thread::Local`,
`Fiber::Local` and `Fiber::Storage::Local` (or something) which correctly clean up on GC.
--
https://bugs.ruby-lang.org/
18 Jan
18 Jan
3:58 p.m.
New subject: [ruby-core:111885] [Ruby master Feature#19333] Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete value in order to avoid memory leaks.
Issue #19333 has been updated by Eregon (Benoit Daloze).
There is also `Thread#key?` for Fiber locals, so not just `thread_variable?`.
Above I am talking more about the performance issue than the semantic incompatibility.
Doing delete+insert is more expensive than set+set, and even much more so if Fiber/Thread
locals are implemented with Shape (which is already the case of TruffleRuby, so not
theoretical).
----------------------------------------
Feature #19333: Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete
value in order to avoid memory leaks.
https://bugs.ruby-lang.org/issues/19333#change-101302
* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As it stands, Fiber Locals, Thread Locals and Fiber Storage have no way of deleting
key-value associations.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
end
```
Because of this, dynamically generated associations can leak over time. This is worse for
things like Threads that might be pooled (or maybe an argument against user-space
pooling).
In any case, having a way to delete those associations would allow application code to at
least delete the associations when they no longer make sense.
I propose that assigning `nil` to "locals" or "storage" should
effectively delete them.
e.g.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
Thread.current[name] = nil # delete association
end
```
A more invasive alternative would be to define new interfaces like `Thread::Local`,
`Fiber::Local` and `Fiber::Storage::Local` (or something) which correctly clean up on GC.
--
https://bugs.ruby-lang.org/
19 Jan
19 Jan
8:58 a.m.
New subject: [ruby-core:111902] [Ruby master Feature#19333] Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete value in order to avoid memory leaks.
Issue #19333 has been updated by matz (Yukihiro Matsumoto).
Accepted, although we foresee some possible issues. One example is that people may use
`nil` as a value when they only care about existence of keys (set-like usage). They should
use non-nil values (e.g., true) for the case.
Matz.
----------------------------------------
Feature #19333: Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete
value in order to avoid memory leaks.
https://bugs.ruby-lang.org/issues/19333#change-101323
* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As it stands, Fiber Locals, Thread Locals and Fiber Storage have no way of deleting
key-value associations.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
end
```
Because of this, dynamically generated associations can leak over time. This is worse for
things like Threads that might be pooled (or maybe an argument against user-space
pooling).
In any case, having a way to delete those associations would allow application code to at
least delete the associations when they no longer make sense.
I propose that assigning `nil` to "locals" or "storage" should
effectively delete them.
e.g.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
Thread.current[name] = nil # delete association
end
```
A more invasive alternative would be to define new interfaces like `Thread::Local`,
`Fiber::Local` and `Fiber::Storage::Local` (or something) which correctly clean up on GC.
--
https://bugs.ruby-lang.org/
28 Dec
28 Dec
8:55 a.m.
New subject: [ruby-core:115954] [Ruby master Feature#19333] Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete value in order to avoid memory leaks.
Issue #19333 has been updated by ioquatix (Samuel Williams).
This is implemented for `Fiber#storage` in Ruby 3.3.
I'll consider expanding it to Thread and Fiber locals as it was agreed on but has a
bigger blast radius.
----------------------------------------
Feature #19333: Setting (Fiber Local|Thread Local|Fiber Storage) to nil should delete
value in order to avoid memory leaks.
https://bugs.ruby-lang.org/issues/19333#change-105911
* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As it stands, Fiber Locals, Thread Locals and Fiber Storage have no way of deleting
key-value associations.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
end
```
Because of this, dynamically generated associations can leak over time. This is worse for
things like Threads that might be pooled (or maybe an argument against user-space
pooling).
In any case, having a way to delete those associations would allow application code to at
least delete the associations when they no longer make sense.
I propose that assigning `nil` to "locals" or "storage" should
effectively delete them.
e.g.
```ruby
100.times do |i|
name = :"variable-#{i}"
Thread.current[name] = 10
Thread.current[name] = nil # delete association
end
```
A more invasive alternative would be to define new interfaces like `Thread::Local`,
`Fiber::Local` and `Fiber::Storage::Local` (or something) which correctly clean up on GC.
--
https://bugs.ruby-lang.org/
126
days inactive
477
days old
8 comments
3 participants
participants (3)
-
Eregon (Benoit Daloze) -
ioquatix (Samuel Williams)
-
matz (Yukihiro Matsumoto)