<headius>
You could check the branch on enebo's fork, I don't think he has landed anything yet
<puritylake[m]>
Will do
<puritylake[m]>
Not noticing anything pertaining to it on his fork, guess I can assume I can pick up from the branch in the main repo
<headius>
Ah ok maybe he did the branch on the main repo
satyanash has quit [Ping timeout: 260 seconds]
satyanash has joined #jruby
satyanash has quit [Quit: kthnxbai]
satyanash has joined #jruby
<enebo[m]>
puritylake: yeah it is still in-progress
<puritylake[m]>
<enebo[m]> "puritylake: yeah it is still in..." <- Awesome I'll get back to work on it so, I'll mention here if I put up a PR for testing
<enebo[m]>
puritylake: great!
nowherefast[m] has quit [Quit: You have been kicked for being idle]
<headius>
when this all lands I'll merge 9.3 to master and we should be back to normal
<enebo[m]>
if that passes on stadium I am happy. There is not much really to review.
<headius>
GHA must be running PR CI against the merged result because I don't know how else this could pick up master changes from a branch based on 9.3
<headius>
perhaps I should know that already
<headius>
this may mean I can reenable spec:ruby:fast for 9.3 on M1 since it should be green
<enebo[m]>
The listen gem example does have maybe an interesting wrinkle
<enebo[m]>
both listen and gdx both essentially run event loops
<headius>
7004 looking good in CI
<headius>
once I merge this I can help with listen thing
<enebo[m]>
I think I solved this problem
<headius>
bravo
<headius>
what is it
<enebo[m]>
I added a gets at the bottom of the file and it works fine
<enebo[m]>
so it was probably trying to finalize or something but it couldbn't because gdx was still running
<enebo[m]>
It does not explain why it works for him for 9.2 but then again it crashes on me on 9.2
<enebo[m]>
ok it works on 9.2 with the gets as well
<enebo[m]>
So it runs off the script and thinks it maybe can shutdown but because we have some daemon threads JRuby stays up but listen tries to cleanup
<enebo[m]>
That explains on 9.2 why I get a conc thread pool error...it decided to try and deliver a change event but listen was already partially torn down
<headius>
aha that makes sense
<headius>
9.3 has different teardown logic that allows more shutdown hooks to run
<headius>
so it might end up swallowing that error in the executor
<enebo[m]>
yeah 9.3 cleanly turns it off
<enebo[m]>
but there is only one unanswered thing
<enebo[m]>
how did you add logging?
<enebo[m]>
Is it possible you created a loop in the main thread?
<headius>
the listen gem has an env var, I mention it in one comment
<headius>
I just set that env
<enebo[m]>
ok perhaps the result of that keeps it alive somehow
<enebo[m]>
I am satisfied this is not our bug but I may see how Lwjgl is supposed to work. I used this a long time ago with jmonkeyengine
<headius>
yeah I could see it changing the sequence of shutdown or doing something to keep it up
<headius>
I agree this is the simplest and most plausible explanation
<headius>
which would mean we have no further work to do other than following up with the reporter
<enebo[m]>
yeah I will write something up and spend a few seeing how you are supposed to do this in Java
<enebo[m]>
It would seem this is missing some loop or something
<enebo[m]>
I guess Java would just exit the thread and others would be running
<enebo[m]>
So maybe that is just a difference here since us leaving this first script makes us think things are done
<enebo[m]>
unless we can mark the thread in such a way we think it is still up
<enebo[m]>
lol sparse docs
<headius>
yeah I have never been happy with this behavior of JRuby but I'm not sure what else we can do
<headius>
all we have to delineate the begin and end of the program is the main script
<headius>
yay, 7004 bash fix on top of GHA M1 fixes is green, merging
<headius>
oh nice k77ch7 has some 3.0 PRs
<headius>
enebo: only thing remaining marked for 9.3.3 is the super NPE thing
<headius>
I will try to repro
<enebo[m]>
ok cool yeah
<enebo[m]>
Does ruby have a Thread.wait?
<enebo[m]>
I think this would be the right workaround here
<headius>
you could sleep
<headius>
not sure what thread you would wait for otherwise
<enebo[m]>
ah yeah sleep with no args is the proper workaround
<headius>
Cannot invoke "org.jruby.javasupport.proxy.JavaProxyClass.getMethod(String, java.lang.Class[])" because "jpc" is null
<headius>
Java::JavaLang::NullPointerException:
<headius>
gotta love helpful NPEs
<headius>
this one may not be too hard to fix
<headius>
it is interpreting a super from within a reopened Java class as though it's a Ruby subclass, but it isn't
<headius>
so it tries to find the Ruby java proxy class and none is there and it NPEs
<headius>
so this is a case we did not cover I guess... adding a method with super to a reopened Java class (without extending from Ruby)
<headius>
enebo: I have a fix that passes spec:ji but it would be good for byteit101 to have a look
<enebo[m]>
great yeah hopefully he is arond
<headius>
not sure if that can happen before we want to release
<enebo[m]>
Could we just use jl.Reflect?
subbu has joined #jruby
<headius>
for what
<headius>
the logic here is checking for a reified Ruby subclass so it can use the super stubs for dispatching properly to the superclass
<headius>
but it is also running for a Ruby method added to a normal Java subclass, which should just use normal Java dispatch to call super
<enebo[m]>
I meant if we knew the Java method on a normal subclass it wanted to invoke couldn't we just make a Reflect call
<enebo[m]>
I am not saying for a final fix but just as a quick fix
<headius>
I have a fix
<headius>
I just detect that there's no reified JavaProxyClass and go back to the other logic
<enebo[m]>
I was confused about your statement about making it happen before we release
<headius>
I meant the byteit101 review
<enebo[m]>
ah well since we crash right now I am not sure how much the review matters for release
<enebo[m]>
I am not saying he shouldn't review it but that atm this is completely broken for this scenario
<headius>
yeah it passes JI specs but I am wondering if it should actually fall into this logic at all
<headius>
but this should fix it assuming it doesn't break something else
<enebo[m]>
headius: you just widened what goes to failure case and when you do that it passes
<headius>
yeah
<headius>
it is a bit of a form-fit fix
<headius>
it's null? ok don't do that branch then
<enebo[m]>
I mean if he comes back and says some subset of that will not work then we still have a problem but you definitely got rid of one case of problem
<headius>
yeah in any case it is working with the patch and clearly busted without it
<enebo[m]>
I am largely just typing to tease out a discussion on risk and to me there isn't any other than getting a better review may uncover a different issue
<enebo[m]>
So if he is not around we roll with it
<enebo[m]>
If he is then perhaps we have that discussion if we have anything to discuss
<enebo[m]>
I think we are good for tomorrow for a release then
<headius>
yeah I think there is a better way to detect this before we try to get a proxy class, but not sure what that is
<enebo[m]>
once these are merged
<headius>
like one of the marker interfaces
<headius>
that's mostly what I want to talk through with byteit101
<headius>
it would still fall back to the logic it falls back to with my patch but wouldn'
<headius>
wouldn't blindly try to get the JPC
<enebo[m]>
yeah so it would just be a cheaper check
<headius>
cheaper and more explicit than "oops it's null"
<enebo[m]>
can we perhaps even emit the helper to directly call the else in this case
<enebo[m]>
at the time code is emitted maybe we will no there is no jpc for it at all
<headius>
while this runs I'm going to try to turn M1 spec:ruby:fast back on for 9.3 on a branch
<headius>
it would be great if we could release with M1 support and say specs are green
<enebo[m]>
headius: I think we can afford to delay a little if we need to since M1 is the main driver for this release
<headius>
modulo those two unix socket specs I did not investigate yet (which may be a Darwin issue rather than M1)
<enebo[m]>
I am very happy we have our marked issues pared down like this. It is satisfying to just get down to zero (I am ignoring the 2.6 issue checklist)
<headius>
this is code I wrote but basically it is trying to use subclass super logic for a normal Java class that is reopened with a super call
<headius>
the bail-out in my patch is if JPC is null just use normal Java dispatch logic
<headius>
just wondering what is the best way to detect that JPC is not relevant before we try to get it
<headius>
(and any other comments you have)
<byteit101[m]>
Ok, need to page that stuff back in . I'll look at it in little bit
<byteit101[m]>
but that reminds me: should I file a bug on the aliasing a java interface method not working with they keyword that I mentioned in december?
<byteit101[m]>
module Address; def +(r);add(r);end;end # works
<byteit101[m]>
module Address; alias :+ :add;end # error
<headius>
not sure if you saw my response but it is expected that would not work
<headius>
our interface modules don'
<headius>
don't actually define any of the methods from the interface because they interfere with dispatch
<headius>
so alias should fail to find it
<headius>
we attempted to make interface modules have stub methods that just super but it causes a lot of problems... interface methods really just do not exist as far as dispatch code
<byteit101[m]>
I saw that, but I guess my expectation was that those two forms are identical on interfaces, and so should be interchangable. So from a users perspective I was surprised they were not
<headius>
yeah alias is different in that it needs to find the existing method to wrap it with a new name
<headius>
since we don't define those methods for interfaces, it errors
<headius>
where the actual def method just redispatches to the proper name, and that method will exist when the module lives within a concrete Java class hierarchy
<byteit101[m]>
would it be useful to have a java_alias or interface_alias, or something like that?
<headius>
it would be a bit like trying to alias `each` to `my_each` in Enumerable... `each` exists in every hierarchy where Enumerable is included, but it does not exist in Enumerable itself
<headius>
yes I was going to suggest that as a possiblity
<headius>
that would be the path forward should we want to make this easier
<headius>
unknown whether adding aliases at the interface module would cause other problems, but at the very least I know this simple alias form can't be made to work
<byteit101[m]>
got it, ok
<headius>
ok spec run on M1 looking a lot better in CI now, but there are still some socket hangs
<headius>
this one is just UDP so it doesn't use any native stuff
<headius>
I went ahead and merged 7007... if there's a better tweak we can still get it in
joast has quit [Quit: Leaving.]
<headius>
no open issues left for 9.3.3
<byteit101[m]>
> just wondering what is the best way to detect that JPC is not relevant *before* we try to get it
<byteit101[m]>
Looking at the patch, that may be fine as is, but I'm not a dispatch expert. All the work I did was for dispatching java to ruby, not the other way.
<puritylake[m]>
enebo: just checking, does the parser support "%<key>s"?
<enebo[m]>
puritylake: I thought so
<headius>
byteit101: you see in the code here it tries to get the JPC from any JavaProxy that enters this logic, but I think there's a better marked for when JPC will actually be present
<headius>
Maybe ReifiedJavaProxy?
<puritylake[m]>
enebo: ok, just checking cause that seems to be what is missing for %s
<enebo[m]>
puritylake: lol. I just pushed four commits from last time I worked on it
<puritylake[m]>
Should I do a pull so?
<enebo[m]>
yeah it will not contain what you were asking about but there are changes to be picked up
<puritylake[m]>
Cool thanks, that'll save me a headache when I go to do a PR lol
<enebo[m]>
puritylake: it looks like the parser is seeing %<name>s and putting name into the name field
<puritylake[m]>
Duly noted, thanks
<enebo[m]>
puritylake: and getArg should retrieve the value using that name
<puritylake[m]>
Awesome
<puritylake[m]>
Just started using emacs so my progress will be slow for now til I get the hang of it lol
<byteit101[m]>
headius: RubyClass#getReifiedJavaClass will return or throw if it's reified java
<byteit101[m]>
I don't think there is an exposed non-throwing one
<byteit101[m]>
Oh oops! that actually is never set
<byteit101[m]>
that should probably be set
joast has joined #jruby
<byteit101[m]>
headius: Yes, JavaProxy#getObject() instanceof ReifiedJavaProxy would also do
<headius>
Excellent, I will have a go at improving this code then
<byteit101[m]>
What would be the place to look for making @ivar be a field access for reified classes?
<headius>
Do you mean manually adding a Java field and using that or leaning on reification stuff to convert it into a field
<byteit101[m]>
reification, so that java (JavaFX really) can set a field and JRubyFX can read it as an @ivar
<headius>
There's nothing explicit right now for defining fields on a Ruby Java class
<byteit101[m]>
IE make the backing store for an @ivar be a java field
<byteit101[m]>
Yes, obviously I'd need to add that
<headius>
The logic for normal Ruby classes just inspects all accessed instance variables and generates fields for them
<headius>
Or really it just picks the right shape class to use, the field names are generic
<headius>
Does JavaFX need to be able to access a field directly?
<byteit101[m]>
the FXMLLoader sets fields with an @FXML annotation
<byteit101[m]>
so wait, hold on, let me dump a class for you
<headius>
Ok
<byteit101[m]>
public class FormattedTableCellFactory extends RubyObject implements Reified, Callback {
<byteit101[m]>
@FXML public TextAlignment alignment;
<byteit101[m]>
@FXML public Format format;
<byteit101[m]>
^ right now jrubyfx generates that, and JavaFX fills the values in, but I must use self.format to access the field
<headius>
enebo: All marked issues are resolved, is there anything else you want me to look at? Otherwise I will pivot back to these M1 socket hangs and see if I can find anything out
<byteit101[m]>
I was hoping to implement something so that @format could read that field
<enebo[m]>
headius: I think we are good to go
<byteit101[m]>
to make it more rubylike
<enebo[m]>
if you do happen to figure those out I think we should consider it
<headius>
Should we aim for Wednesday perhaps?
<headius>
I will spend a little time on these today
<enebo[m]>
headius: sure we can or tomorrow even but that depends on whatever progress you make
<enebo[m]>
headius: but perhaps it takes some pressure off for us to just target wednesday
<headius>
byteit101: The logic that handles using fields for instance variables mostly centers around VariableTableManager
<headius>
For ruby classes with reified fields it uses a special form of variable accessor that knows how to go after the field rather than the instance variable table
<headius>
So the general pattern would be a new sort of field accessor that can access a specific name and then we wire that into the table manager for reified Java classes
<headius>
And then the rest of the magic is in the jit so it knows how to go straight to the field rather than through the accessor
<headius>
The variable table stuff could be made more generic because right now it really just supports these two types of accessors
<headius>
And we want to make it more generic going forward so that we can use a different layout for objects as they evolve, like if you add another instance variable later we will choose a different shape from then on
<headius>
Right now it only chooses the shape when you first instantiate the object so it is a guess as to which instance variables will be in use
<headius>
I don't have the code in front of me at this exact moment but I believe the accessor is FieldVariableAccessor
<headius>
enebo: whenever we get around to this it would align our object shaping with certain other "out of our league" implementations that can evolve the shape over time
<byteit101[m]>
// This alternate ivar logic is disabled because it can cause self-referencing
<byteit101[m]>
Where can I go to look up the old issue descriptions?
<byteit101[m]>
// chains to keep the original object alive. See JRUBY-4832.
<byteit101[m]>
from before the move to Github
<headius>
You can't
<byteit101[m]>
:-(
<headius>
In retrospect I wish we would have pulled a dump of all of those issues but that train has sailed
<headius>
And it would have been in some horrible format or raw database anyway
<byteit101[m]>
hmm... ivars are assumed to be an IRO, what can I use to wrap that
<headius>
hmm
<headius>
shouldn't be
<headius>
ahh well the class-level methods for getting ivars do assume IRubyObject
<headius>
internal vars API works with Object though
<headius>
in order to allow accessing fields as normal ivars I think we would have to EITHER force the stored values to be the wrapped proxy object OR override logic for accessing ivars from these classes to wrap the value lazily
<headius>
having non-IRubyObject in Ruby ivars is not really supported