[feat] support clearing of thread local state#8369
Conversation
kares
force-pushed
the
thread-clear-locals
branch
from
c8cf998 to
8c7ebca
Compare
| return RubyString.newString(runtime, rubyName); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
I think I would be interested in seeing this fall into org.jruby.api somewhere. I have been trying to get all methods people would use in native extensions into this package space. I have also been dogfooding our internals to use these same APIs when it makes sense.
org.jruby.api.Runtime#currentContext(Ruby) or something like that? Not sure on either the class name or the method name but just putting that out there.
The other broad point is to kill off any external consumers from hitting Ruby methods at all. I would love to never see us use Ruby as a reference anywhere some day but I think needing to get current ThreadContext will be with us for a long time.
There was a problem hiding this comment.
Just to add here that 9.4 has none of the new .api packages but the plan is to backport those files so we have a migration path for 9.4 and possibly 9.3 for native extensions.
There was a problem hiding this comment.
I would love to never see us use Ruby as a reference anywhere some day
it's a multi-tenant runtime so I am not sure how that would end up being achieved...
what I can tell, as I've been on multiple bigger projects consuming and mixing Ruby and Java is that the runtime is usually something that is kept externally or queried frequently Ruby.getGlobalRuntime (even if it's a single runtime use-case).
seeing this fall into
org.jruby.apisomewhere
would be nice, but it's a big chunk IMHO, the only way to cleanup the distinction would be to have interfaces on objects that Ruby users from Java encounter e.g. RubyArray implements IRubyArray, RubyHash, RubyThread etc. having just the one IRubyThread added here, with a static method, doesn't get us far...
I can remove the extra current(final Ruby runtime) in this PR and can continue consuming current(IRubyObject recv), it's mostly for convenience
There was a problem hiding this comment.
@kares yeah for embedding with multiple runtimes you have to use runtime. I meant for internal use and java native extension gems. Even then I know we will still need something to get TC from runtime.
Also I am only talking about this single method current so we don't have another way of getting TC. If you just move this to a new class with a static method: org.jruby.api.VM.threadContext(Ruby runtime) then I think it is fine.
There was a problem hiding this comment.
I've reverted the change for now (the work-around with RubyThread.curren(IRubyObject) will do, it's likely a minor use-case anyways).
I think I get what you mean but I believe it deserves a proper API design, just moving a static method somewhere while still returning RubyThread sounds a bit half-baked and I can not commit to the full thingy atm.
|
marked as 9.4.9.0 would be nice to have it but feel free to post-pone... |
|
I'm +1 to merge. |
3 participants
this is to support cases where e.g. a Java thread pool is running Ruby code.
if the ruby code uses any fiber/thread local state e.g.
Thread.current[:foo] = :barthan there isn't an easy way to cleanup accumulated state in RubyThread's locals (and due the way foreign Java threads are adopted the state will stick around for when the Java thread is re-cycled).