<headius>
What about enebo's 's idea to just add native_thread (and getNativeThread aliases) to java.lang.Thread with a deprecation warning?
<enebo[m]>
monkey patch!
<headius>
So far all cases we have seen immediately try to get the native thread from the JRuby object, so we could warn that this pattern is deprecated and you can just to_java now
<enebo[m]>
not even sure if I care about deprecation as long as no one can observe visibility of the change
<headius>
I'd rather not have that duck typed patch in there forever though
<enebo[m]>
yeah ideally I don't either but in practice it tends to be how it goes
<headius>
The point of the deprecation is to get people to just use to_java though
<headius>
If there's no warning then we'll definitely never be able to get rid of those methods
<headius>
Of course we're talking about celluloid and I'm not sure it's actually maintained so we'd be opting into deprecation noise until they change it, and then it won't work on 9.3
<enebo[m]>
yeah that's fine.
<headius>
what's fine
<enebo[m]>
adding the deprecation
<kares[m]>
yy did consider enebo suggestion around adding native_thread getNativeThread on j.l.Thread but it feels less "clean"
<kares[m]>
having it deprecated should have been the way forward in the first place - if I would have checked actual usage out-there on GH 🤯
<headius>
so let's review... the options are to revert the default to_java and leave it as RubyThread (forever?), or to keep it as j.l.Thread with native_thread aliases to be backward-compatible (possibly with deprecation warning)
<headius>
the former would remain compatible with all actively-used JRuby versions but we'd probably never get rid of this pattern in the wild
<headius>
the latter isn't exactly compatible but it's duck-type compatible enough and the deprecation would encourage users and libraries to just use to_java without native_thread
<headius>
so I guess the questions I have are: what's our end goal with this and how long do we need backward compat on this change
<kares[m]>
having Thread#to_java with a deprecation (that I am planning to follow up with) will get rid of it eventually why do you mean we won't get rid of it?
<headius>
it's largely accidental that to_java just returns the JRuby object (and we might want to reconsider that as a default in general)
<kares[m]>
yeah it's unfortunate but this RubyThread case had a valid use-case out there ... more than the others
<headius>
well this is my problem with adding deprecation warning to to_java(): it will be unavoidable as a warning until we actually break it and go to j.l.Thread
<kares[m]>
nope - the way forwards to libraries will be to to_java(org.jruby.RubyThread) which will work in all versions (planning to follow up with the libraries to use that)
<headius>
so it gives people incentive to stop expecting RubyThread but does not give them a warning-free alternative
<kares[m]>
if they rely on internals - make it explicit
<headius>
other than explicitly to_java(j.l.Thread)
<kares[m]>
> other than explicitly to_java(j.l.Thread)
<headius>
if we just add native_thread etc to j.l.Thread with a deprecation warning, they can move to to_java() and get native thread without a warning
<kares[m]>
the only problem there is that wouldn't be backwards compatible
<headius>
correct
<headius>
nor would moving to to_java() and expecting j.l.Thread
<headius>
something will have to break clearly
<kares[m]>
okay but what if someone relies on smt else than getNativeThread / native_thread
<headius>
nor would moving to to_java(j.l.Thread)
<headius>
yeah that's a possibility but do we know of any examples?
<kares[m]>
nope and there probably aren't but if there is this ends up a bit messy ...
<headius>
it is messy for sure
<headius>
I mean, least impact is just adding to_java(j.l.Thread) and no deprecation
<kares[m]>
oh so there's an actual use-case: Thread.current.to_java.getContext()
<kares[m]>
this is out there :yay:
<headius>
yeah so if we wanted to cover this we are now adding a bunch more aliases with deprecation
<kares[m]>
need to think about this a bit more - I do not like having to add java.lang.Thread#getContext that much
<headius>
the only pattern that will keep working on both 9.3 and 9.4.1+ is the duck typed solution
<headius>
yeah it isn't as logical as adding def native_thread; self; end
<headius>
so answer this question: is our goal to finally make to_java() return j.l.Thread?
<headius>
if that is the goal then there's no perfect options wrt compat and warnings
<headius>
it is a breaking API change no matter what
<headius>
when we added to_java we probably should have added a logical default Java type for most code classes and error for any that don't have one
<headius>
rather than exposing internal objects
<headius>
but that train has sailed
<kares[m]>
> our goal to finally make to_java() return j.l.Thread?
<kares[m]>
my goal was to have a non-internal way of getting a Java thread from a Ruby one
<kares[m]>
having it as a default just made sense ...
<headius>
it does make sense, I'm just trying to weigh whether it makes so much sense we should break the API (now or later)
<kares[m]>
but I can get it with to_java(java.lang.Thread) now so if the default requires monkey-patching java.lang.Thread I am a bit hesitant about whether its "worth it"
<headius>
it is not heavily used, we know that much... but it is used in the wild
<headius>
yeah I am leaning that way too
<kares[m]>
but you do not want a deprecation with default Thread#to_java() (no args) right, cause that is unfortunate?
<kares[m]>
I mean returning the same as 9.3 (the internals) with a deprecation
<headius>
I just don't like deprecations you can't avoid, and if you want backward-compat with 9.3 you would have to accept the warning
<headius>
plus it doesn't get people ready for to_java() => j.l.Thread because in order to avoid warning they would have to add to_java(j.l.Thread)
<kares[m]>
yy understood
<kares[m]>
happy to leave that undecided for now and in the mean-time cleanup to_java usage in gems out there
<headius>
here is perhaps a halfway option: deprecate RubyThread#native_thread from the Ruby side
<headius>
actually nah, that doesn't help us much
<headius>
because the getContext pattern would not warn
<headius>
yeah if we submit some patches to the ones we know about to use to_java(j.l.Thread) we might eventually be able to make it default
<headius>
once this pattern has been available for a full release
<headius>
like just add j.l.Thread support now, help libraries fix it, and then deprecate to_java() in 9.5 and make it j.l.Thread by default in 9.6
<headius>
or whatever those versions are called
<kares[m]>
yeah I guess that would be the ideal path ...
<headius>
and the code out there using getContext can switch to to_java(RubyThread) in the short term and remain backward-compat
<headius>
or we provide a less-hacky way to get context
<headius>
JRuby.context
<kares[m]>
most uses out there will have to be simply to_java(org.jruby.RubyThread) in order to be backwards compatible - to_java(java.lang.Thread) does not work in 9.3
<kares[m]>
but the good news is with the Thread.current.to_java.getNativeThread pattern they can be switched to simply: java.lang.Thread.currentThread.
<headius>
yeah true
<headius>
enebo: so I think we have a path forward
<headius>
1. to_java() returns to RubyThread by default
<headius>
2. we fix known cases of to_java.native_thread to use to_java(j.l.Thread) or j.l.Thread.current_thread
<headius>
3. next major we add a deprecation warning to `to_java()` encouraging either to_java(RubyThread or to_java(j.l.Thread) and deal with complaints by helping to patch libraries
<headius>
4. subsequent major we switch `to_java()` to j.l.Thread
<headius>
fun stuff
<enebo[m]>
sounds reasonable
<headius>
so no deprecation warning or breaking API change until "9.5"
<headius>
and breaking API change really waits until "9.6"
<headius>
kares: if you can take care of reverting the to_java default I will file some issues for 9.5 and 9.6 for the other steps here
<kares[m]>
yep will do that in the test PR that's already out there to also re-adjust the test
<kares[m]>
it's relying on that to extract file/line from Proc
<kares[m]>
seems a bit far off but I haven't looked into it ...
<headius>
ok
<headius>
I am just adding them as tasks for the deprecation issue
<kares[m]>
okay - these usages aren't that scary expcet the method_source
<kares[m]>
- sigdump seems very narrow and could be fixed easily
<kares[m]>
- celluloid is unfortunately still dead (died twice already)
<kares[m]>
but there might be more I haven't found and I bet most JRuby (on Rails) shops using JI also does a similar thing as it's more useful to set a Java thread name than a Ruby one
<headius>
if we submit PRs to dead projects at least people will find them when we add the deprecation, even if they don't get merged and released
<headius>
so they can monkey-patch if necessary
<headius>
and for a dead project we still get bug reports when we break Celluloid (as we did in 9.4)
<enebo[m]>
Does tony still have any control?
<enebo[m]>
maybe he could do a one-Pr release
<headius>
I'm pretty sure he completely handed it off to others
<enebo[m]>
yeah I remember that happening but he may still be able to do it
<enebo[m]>
it is a big ask even if so
<kares[m]>
he spent so much time handing it over and trying to help with the resurrection that I do not think he will be happy doing this
<enebo[m]>
funny that as a project their release stride is very long
<kares[m]>
esp. with all the release branches
<enebo[m]>
It could be more of a "finished" project at this point