Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

thread.object_id with fibers (due AR's connection pooling) #1717

Closed
kares opened this issue May 28, 2014 · 4 comments
Closed

thread.object_id with fibers (due AR's connection pooling) #1717

kares opened this issue May 28, 2014 · 4 comments
Milestone
Invalid or Duplicate

Comments

@kares
Copy link
Member

kares commented May 28, 2014

MRI (in 2.x) does return the same object_id for Thread.current inside a Fiber ... this is not how JRuby behaves (even on master). it's quite important esp. due the fact that Rails uses Thread.current.object_id as it's key to store/resolve DB connections thread-locally using a pool ... they seem to not be doing anything about it - thus decided to re-report the issue here rails/rails#7746

Rails might switch to using thread_variable_xxx but this is also not currently available on JRuby (2.x mode) - which might be another approach to handle this right for structures such as thread mapped connection pools. MRI 2.1 samples :

Thread.new {
  Thread.current[:foo] = "bar"
  p [Thread.current.object_id, Thread.current[:foo]]

  Fiber.new {
    p [Thread.current.object_id, Thread.current[:foo]]
  }.resume
}.join
[23336800, "bar"]
[23336800, nil]

Thread.new {
  Thread.current.thread_variable_set :foo, "bar"
  p [Thread.current.object_id, Thread.current.thread_variable_get(:foo)]

  Fiber.new {
    p [Thread.current.object_id, Thread.current.thread_variable_get(:foo)]
  }.resume
}.join

[23281980, "bar"]
[23281980, "bar"]
@headius
Copy link
Member

headius commented May 28, 2014

Honestly, the MRI behavior just looks completely broken to me. Thread.current supposedly returns the same object, but Thread.current[:something] is per-Fiber. But I know this has been argued to death and it isn't going to change.

I have been reluctant to make thread-local data pretend to be like MRI. It ends up getting very messy internally since we still need to model fibers as threads. We also use Thread.current to get the thread object associated with the running fiber...not the parent thread, but the actual Ruby thread that the fiber creates. If we were to start mucking around with Thread.current, that logic would have to know about it. I made a previous attempt and ended up breaking fibers badly.

So I'm not sure what to do here. We could ship a JRuby with thread_variable_get exposed in all modes; it would be exposing a 2.x feature in pre-2.x mode, but I'm guessing the Rails logic would just start working. That seems better to me than trying to fake out Thread.current, which has a lot of nasty implications.

@kares
Copy link
Member Author

kares commented May 28, 2014

@headius implementing thread_variable_get in the 1.7.x sounds very reasonable for this madness :)
I'll try to look at the other side - at AR so that they flip over using "true" thread locals (at least) in the next 4.2

@chuckremes chuckremes mentioned this issue Feb 3, 2018
Open
@chuckremes
Copy link

This bug is still causing some headaches. See issue 2 in ruby-io for detail.

@headius
Copy link
Member

headius commented Feb 13, 2018

Same issue as #1806. Root issue is that Thread.current in a Fiber is not really the parent thread but the Fiber's thread. Many synchronization mechanisms depend on these being the same, so simply changing it is not easy.

@headius headius closed this as completed Feb 13, 2018
@headius headius added this to the Invalid or Duplicate milestone Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Invalid or Duplicate
Development

No branches or pull requests
3 participants
@headius @chuckremes @kares