<headius>
the issue here is that the block with the break is still live; the break would be valid if it were called in the same thread, but since it is not it bubbles out of the thread and only gets reported as an internal thread termination exception
<headius>
we can catch these flow control exceptions right before they terminate the thread, turning them into LJEs, but that doesn't match CRuby which propagates them as LJE immediately
<headius>
I'm not sure where to look in CRuby to figure out how they determine this is an LJE right away
<headius>
so this is another case where they are actively scanning live frames on this thread to see if they match the target
<headius>
so we need another way to detect that the block's parent scope does not live in this thread, or we just punt this and assume raising the right error in the end is good enough
<headius>
ok so that search above find the the first non-local scope (method scope) starting from the block, and then a subsequent call to rb_vm_search_cf_from_ep scans the current stack looking for the matching scope, or else returns null
<headius>
the null then indicates this should raise as LJE
<headius>
so they basically walk up the scope tree from the block AND up the current thread's scope stack every time
<headius>
some other maybe-not-feasible options...
<headius>
* store a reference to the home ThreadContext in the block and if they don't match at `break` time raise error... might keep TC alive longer than it should be, though
beholder82 has joined #jruby
<headius>
* same but store and compare some unique Thread ID... better but still makes the block data a bit larger
<headius>
* same but on the scopes, also making them bigger... we would detect that the target scope belongs to a different thread
<headius>
fair bit of plumbing required to add a thread reference to every scope
<headius>
we construct them through generated code as well
beholder82 has quit [Quit: Ping timeout (120 seconds)]
beholder60 has joined #jruby
<beholder60>
hello people — is this an appropriate place to ask a "D'ya think my application will benefit from JRuby?" question or are you just about JRuby internals here?
MattWelke[m] has joined #jruby
andreimaxim[m] has joined #jruby
<enebo[m]>
headius: when you mean scopes which ones in particular?
<basshelal[m]>
headius: Been looking into the jffi building mechanism, I think most platform-arch combos should be ok with qemu, the only major issue would be AIX which you'd need a license from IBM to get a download from
<headius>
enebo: target scope for a break
<enebo[m]>
IR Scope?
beholder60 has left #jruby [#jruby]
<headius>
DynamicScope
<headius>
the intent is to know whether the target scope lives on this thread without walking stack
<enebo[m]>
ah ok good...just checking since the chore of removing runtime info from IRScope in 2019
<enebo[m]>
seems like some features just cause us to add fields so YOLO
<headius>
yeah wouldn't make sense to have thread info on there anyway
<headius>
dynscope having a thread ID or reference would make them bigger but we'd always know whether we are using a scope from current thread or not
<enebo[m]>
There is another problem that during execution MRI also tracks whether we are in topself
<enebo[m]>
for extra warnings/errors but for some IR instructions I think we end up with the receiver of Object
<enebo[m]>
This is unrelated to what you are talking about but it is something (I think very minor from compat standpoint) I noticed
subbu has joined #jruby
<enebo[m]>
The memory of which specific things need that I don't recall so I am not sure about remedy (but dynscope seems one possibel one)
<headius>
that should be easy enough for us to detect too
<headius>
there is the simplest fix for this thread/break issue but it is not possible to rescue the LJE within the thread
<headius>
enebo: another option would be to only add thread to scopes that are break targets, since we can do that statically, at least saving us setting it up (but probably not saving size unless we're going to generate two sets of scopes, with and without thread)
<headius>
FWIW this thread ID on scope might also help us address threaded $~ and $_ overwriting each other... we could at least detect use across threads and warn or take some action to fix it
<headius>
oh though we keep that on frame, not dynscope 🤔
<headius>
so there's a naive version of this just using a unique Object per ThreadContext to identify which thread a given dynscope was created for
<enebo[m]>
early lunch
<enebo[m]>
back now
<headius>
enebo: welcome back
<headius>
I'm going to make a sammy, let me know how that patch makes you feel
<enebo[m]>
I feel funny inside
<enebo[m]>
For original PR I think I am ok but I am not sure I can foresee what could break from it
<enebo[m]>
9.3 itself is still young enough where we will have a few weeks to tease out any issues if they exist
<enebo[m]>
In other news shoveling every day sucks
<headius>
buy a snowblower
<enebo[m]>
I have one
<enebo[m]>
but 1" of fluffy snow is not worth it
<headius>
I probably wouldn't shovel that either
<enebo[m]>
I should have did the saturday with a snow blower but I just did it by hand
<headius>
is it done snowing today?
<enebo[m]>
yeah I think it was only last night
<enebo[m]>
we have some really icy areas we have to keep clean and since we have a lot of deliveries I try and stay on top of it
<headius>
so regarding this break thing
<enebo[m]>
but it is weird to snow so many consecutive days
<headius>
the PR should be completely safe... it just avoids the break jump from falling off the thread and not being reported as a Ruby exception
<headius>
the thread ended either way but now it will have a proper join/value to indicate it terminated prematurely
<enebo[m]>
so long as stuff like getException only is called by these endy-leafy areas then it won't change the exception
<enebo[m]>
The other place is at that the edge itself
<enebo[m]>
I am just randomly commenting and not implying any problem
<enebo[m]>
I am hopefully finishing up find pattern (which I thought I had already implemented). It is still experimental in 3.1 but I figured it may as well get in since it is a basic language feature
<headius>
This is a weird enough case I don't feel inclined to fix it in 9.2 but it seems pretty safe for 9.3
<headius>
The example case is very synthetic so I think I will ask the reporter if they ran into this in a real world situation
<enebo[m]>
yeah the example sort of looked like it was a real issue but perhaps this is another carrot if anyone else runs into it
<headius>
I did not test the behavior of other jumps
<enebo[m]>
break and next see to follow each other
<headius>
This is the sort of issue that I fix reluctantly because nobody should be expecting a break across threads to do anything specific since it's just an invalid operation
<enebo[m]>
yeah
<headius>
next should not be a problem because it only returns from the block
<headius>
I merged in the simpler fix and resolved the issue with a question about how the reporter discovered this behavior
<byteit101[m]>
So is there anything in particular I need for the 7012 patch to wire it up to JIT or am I all good?
<byteit101[m]>
(ivars in raw fields)
<headius>
You should look at where the field variable accessor is referenced directly as part of the jit, basically it's just another branch in the instance variable accessor logic so it knows how to wire directly to the contained handle
<headius>
It occurs to me now maybe you extended that class and it will be picked up as is?
<headius>
I had to skim your PR on my phone so I might have missed that
<byteit101[m]>
Yes, that's why I went with an extended class :-)
<byteit101[m]>
and chained the getter and setter with binder
<headius>
Then you're probably right, there shouldn't be anything more necessary
<byteit101[m]>
cuz I was hoping it would do it for me automatically
<headius>
With your patch we could also consider expanding class reification to use appropriately named fields for regular ruby classes, should that be useful to someone
<headius>
Mainly it seems like it would be useful only for debugging purposes but as in this case perhaps also when passing a Ruby object to something that needs to reflect against its fields
<headius>
I don't know of such a use case right now
<byteit101[m]>
that already is configurable, this is mostly for java interop though
<headius>
Ok cool
<byteit101[m]>
Actually that's the test I think, no concrete inheritance
<headius>
I will do a more complete review now if you are happy with it
<byteit101[m]>
see the checklist at the top. I have some basic tests, but unsure if should add a jit test? (also unsure how)
<byteit101[m]>
need to change the configuration name
<byteit101[m]>
I need to consult with you for exception types
<byteit101[m]>
and ensure clones/parents/frozens are taken care of
<headius>
A test in spec/compiler/general_spec.rb will get run against all possible configurations
<byteit101[m]>
Java Object (concrete extension) and Ruby BasicObject can't be frozen, correct?
<headius>
Various interpreter configurations plus jit with and without invoke dynamic
<byteit101[m]>
I have tests in j_i
<headius>
That's probably fine, I was just pointing out if there was anything that really is relevant to the compiler it should go there
<headius>
It's really just an extension of the existing field support which is always on and has been tested by stuff in there for a while
<byteit101[m]>
I put it under become_java as it requires reification
<byteit101[m]>
Only Ruby Object decendents can be frozen, or are there some exceptions?
<byteit101[m]>
(also, I'm pleasantly surprised how short this PR is compared to the concrete stuff :-P)
<headius>
BasicObject and down can be frozen from Java side but I think the Ruby methods are only on Object
<byteit101[m]>
So any non-concrete IRubyObject should have .invokeVirtualQuiet(RubyObjectSpecializer.LOOKUP, "ensureInstanceVariablesSettable"); checked?
<byteit101[m]>
(see FieldVariableAccessor)
<headius>
Non-concrete as in not extending a Java class
<byteit101[m]>
Correct
<headius>
Yes
<byteit101[m]>
and java extensions can bypass that check
<byteit101[m]>
?
<headius>
Yeah that's an interesting point, we have the ability to also freeze just the Ruby subclass parts but is that useful
<headius>
There's really no concept of delayed final in Java
<byteit101[m]>
Thus @foo=test errors, but self.foo = test works
<byteit101[m]>
which is freaky
<headius>
Ha yeah
<headius>
That is a bit unexpected
<headius>
I'd say skip it for now but we should formalize what freeze means for Ruby Java subclasses
<headius>
It's troublesome because we can never fully support it since we can't freeze the superclass fields but that means there will always be a behavioral difference for Ruby classes that simply implement an interface and can be fully frozen
<byteit101[m]>
Actually, that self. vs @ extends to ruby-reified classes too
<byteit101[m]>
wait
<headius>
The simple answer may be that freeze only effects the fields declared in Ruby and leave it at that
<byteit101[m]>
Yup, ok self.field= is not affected by freeze on `class A; end.become_java!`
<byteit101[m]>
no java heirarchy
<headius>
heh yeah
<headius>
no way we can prevent writes to the Java fields from other Java stuff
<headius>
unless it's final to begin with, which is hard to do with Ruby's alloc+init dance
<byteit101[m]>
split supers ? :-D
<headius>
a step in that direction to be sure
<headius>
harder to make initialize into a real constructor for regular Ruby classes that might overwrite it or decorate it
<headius>
we'd have to be able to make some guarantees about how the class will change in the future
<byteit101[m]>
> Thus @foo=test errors, but self.foo = test works
<byteit101[m]>
^ happens today with reified ruby classes, but that's because they are two separate variables
<byteit101[m]>
thus if you have a class, and become_java it, it becomes unfreezable for all java_field's defined
<headius>
yeah
<headius>
tricky
<byteit101[m]>
should configuring the ivar storage wipe the default field accessors? forcing field access to go through the ivar?
<byteit101[m]>
or re-set them with attr_accessor ?
<headius>
hmm possibly
<byteit101[m]>
(I've pondered with naming it java_attr_* to configure stuff)
<byteit101[m]>
(but that confused the matter in my mind)
<headius>
when are we adding the default field accessors in this case?
<headius>
enebo: you have any thoughts on this?
<headius>
it is a dichotomy we have had forever
<byteit101[m]>
JavaClassAddons. part of the become_java! call
<headius>
I feel weird about allowing the Ruby part to be frozen when we can't freeze the Java superclasses, but it also feels weird to not allow the Ruby part to be frozen
<headius>
byteit101: I think we'd want to just insert the right accessors at that point then
<headius>
if we decide that fields configured for a Ruby class should be freezable, that is
<headius>
the simplest distinction here is that if you want Ruby freeze semantics it needs to be a variable governed only by Ruby semantics
<headius>
if you make it into a non-final Java field then it's always going to be a non-final Java field and we can no longer enforce freeze semantics
<headius>
I think it would be misleading to have it prevent modification from Ruby but be modifiable from Java since that would represent a possible exploit path
<byteit101[m]>
A fair point of caution to users: "using java_field + ivar storage is an advanced use of jruby for integrating with the jvm more rightly. Ruby semantics no longer apply in such advanced usage of jruby"
<byteit101[m]>
*tightly not rightly (though some may beg to differ :-D)
<byteit101[m]>
"JVM semantics take precedence over ruby semantics in such usage"
<headius>
I added some comments but it looks fine
<headius>
yeah the next complicated feature we will want to add will be "I really want these ivars to be in final fields set in my reified constructor"
<headius>
in which we would emit the initialize of a fully-Ruby reified class as its actual constructor so it can set finals
<headius>
kinda has to be an opt-in since it changes the semantics of initialize
<byteit101[m]>
I think I laid the groundwork for that in the concrete refactor
<byteit101[m]>
cool, thanks!
<byteit101[m]>
what's the best ruby exception to throw for the binder exceptions?
<headius>
Yeah it can definitely happen in the concrete subclass stuff and might actually be easier to do in reified Ruby classes
<headius>
Since we don't have that split initialization problem
<byteit101[m]>
re the lambdafication: I really wanted to rename the "get" because it was always "Get or make"/"ensure", but didn't want to touch too much
<headius>
Really the only guarantee we need, which the user will opt into, is that the initialize is really the constructor and always will be
<headius>
Yeah this is fine for now but I wouldn't want to add too many other accessor types without formalizing an interface
<byteit101[m]>
if you check the jvm output, storage of Object vs some java class should inline differently
<byteit101[m]>
I'll work on the cloning and parent stuff this evening
<headius>
Ok
<byteit101[m]>
(because the object doesn't need to be unwrapped from an irubyobject)
<byteit101[m]>
Oh, and: what's the best ruby exception to throw for the binder exceptions?
subbu has quit [Ping timeout: 256 seconds]
<byteit101[m]>
oh and confirmed false: @foo.equal? @foo
<enebo[m]>
Hmm
<enebo[m]>
I like the idea of configuring how we store field values reified as either IRubyObject or Java Object universes so if you know how you will be accessing those fields you can have it set to the way you need it (e.g. lots of reads from Ruby you probably want IRubyObject)
<enebo[m]>
The duality of us trying to unify @ with 'this.field' I feel less certain on how it would be if we boiled the ocean
<enebo[m]>
by boiling the ocean I think I mean in writing a complete specification on how these two generally separate things alias over each other
<enebo[m]>
Have we ever talked about making a 'this' method?
<enebo[m]>
@foo is Ruby and will be Ruby
<enebo[m]>
this.foo will be Java field and only a Java field (but potentially any object including IRubyObject)
<byteit101[m]>
yes I have a todo on how to expose an "unwrap parameter". ex: `java_field "blah", variable: {unwrap: true, type: java.lang.Object}`
<enebo[m]>
That specifies how it is stored in reified class which is fine
<enebo[m]>
I am talking more about how to access the field in Ruby
<enebo[m]>
@blah can be how this works but this.blah could also be how it works
<byteit101[m]>
Yes, Oh context for why I'm playing around with this: JRubyFX/JavaFX integration
<enebo[m]>
yeah
<byteit101[m]>
lots of times to have to extend java classes, and access their fields (particularly with fxml)
<byteit101[m]>
and I was hoping to make it more ruby-esq
<enebo[m]>
Can reified types still include Ruby modules?
<byteit101[m]>
(which convieniently will avoid breaking api in jrubyfx, as it uses @ right with the "old" fxmlloader pre-concrete impl in jruby)
<enebo[m]>
I am only bringing up this idea to tease through what ups and downs exist
<byteit101[m]>
Yes, that's how jrubyfx works, and I know it works with reified types
<byteit101[m]>
yes, it's a good thing to do
<enebo[m]>
having a syntax for fields of Java and Ruby as separate eliminates some potential questions on how to handle the overlap but it is clearly a big change
<enebo[m]>
So if there is no @foo as a field it will be a Ruby ivar but if there is one then it becomes a Java field and then all reflective methods in Ruby have to deal with this I guess
<enebo[m]>
instance_var_get for example
<byteit101[m]>
(only if you configure it as such, this is non-default behavior I'm adding)
<byteit101[m]>
@foo and self.foo are different unless a) it's reified, b) you have a java_field, and c) you enable this feature on that specific field
<enebo[m]>
ok this makes sense to me
<enebo[m]>
especially if it is explicit in the definition of java_field
<enebo[m]>
or explicit in seeing there is a definition of java_field if it makes sense to default @foo to field
<byteit101[m]>
with the default being variable: false
<enebo[m]>
I don't have an opinion on it and I can see variable: false probably breaks nothing
<enebo[m]>
well I don't yet
<byteit101[m]>
I'm slightly more in favor of `bind` or `bind_variable`, but yes, the goal is to change nothing with the defaults
<enebo[m]>
Ok well my current thoughts are explicit java_field that configures your preference is making your funeral/party. It is a visible line
<byteit101[m]>
this is only for opt-in for advanced java-ruby integration
<enebo[m]>
As such defaulting one way or another is just ergonomics and we perhaps will be ok with more setup to not break existing code
<byteit101[m]>
yes, I'm happy with that. see the "proposed" name in the PR (7012)
<enebo[m]>
So the second idea but I believe we decided this a long time ago when I made the java signature parser was how to specify the field
<enebo[m]>
I think at that time the string form like you have in the above example had the advantage we could just paste in javadoc signatures
<enebo[m]>
The other syntax was to make an API for this stuff like java_field java.lang.String, :mystrfield, variable: true
<enebo[m]>
I am not sure why I am bringing this up other than seeing we are using java signature parser and trying to remember how those decisions happened
<enebo[m]>
One thing I do remember is annotation parsing is surprisingly potentially complicated (so complicated I don't think that work ever got 100% completed)
<enebo[m]>
That made the String format more appealing or we would need some API for building the expressions of annotations