jswenson[m] has quit [Quit: You have been kicked for being idle]
<headius> Good morning
<headius> enebo: so we I guess we broke something
<enebo[m]> the new issue?
<headius> and by we I mean I
<enebo[m]> Let's just revert this and re-release now
<enebo[m]> Make a new issue to fix it properly for
<enebo[m]> err :)
<headius> heh
<enebo[m]> I am willing to wait and do this for Monday in case we get another report I guess
<enebo[m]> I will just take tuesday -> tuesday off
<headius> yeah I'd be more comfortable giving it today and looking into the problem
<headius> it works fine for Set alone but I would guess it is not getting frame info to redispatch properly
<headius> so it keeps supering into Set from the subclass
<enebo[m]> ok. I think we can revisit the risk cost of reverting vs pushing forward
<enebo[m]> based on how obvious the issue is
<enebo[m]> I have fixed so many printf issues that we have had forever in our printf implementation
<enebo[m]> It makes me think most features of printf are not actually used by any one
<headius> wouldn't surprise me
<enebo[m]> Also we use BigInteger and I would think we should match MRI but we seem to give perfect endless precision and MRI gives inexact results
<enebo[m]> So in those few cases I feel we give a better result :) so I tagged those out
<enebo[m]> For floating point or something I can see this more but for integral values I don't really get it
<headius> no easy workaround for this
<headius> no easy fix for the original issue that doesn't have this problem either
<headius> moving initialize_dup/clone into Ruby would fix the super problem but then it needs to call into Java Set internals
<enebo[m]> Is this really also on jruby-9.2?
<headius> it would appear to be in 9.2.20 also
<headius> so it has been out there for a while now
<headius> class MySet < Set; end; class MySet2 < MySet; end; MySet2.new + MySet2.new
<enebo[m]> Is that what is happening?
<headius> MySet alone is not enough
<enebo[m]> oh wait I misread that
<headius> concurent-ruby's Set extends JRubySet (with Synchronized mixed in) which extends JRuby's Set
<headius> so my patch worked fine for the original report, which only had one descendant, but it overflows with two descendants
<headius> because it naively acquires the class
<headius> simplest fix is to just force a frame for these Set#initialize_dup/clone and use frame class again
<enebo[m]> so the super is actually two down but you pick the one immediately below and that causes it to find itself again?
<headius> yeah
<headius> this fix might actually be fine
<enebo[m]> This is because we do not push a frame so it sees current?
<headius> yeah I wanted to avoid the frame push for init_dup but it can't be avoided if there's deeper subclasses
<enebo[m]> If we push for these methods then we will see the right thing
<enebo[m]> ok
<headius> I should have a fix in a moment
<headius> longer term we can avoid framing for super if I push the target class and method name through invokers into the Java impl
<headius> but for now it will only work in cases where we know there aren't descendants
<headius> the frameless way to do this would be to always super from Set here, since we know this method is defined in Set, but we have not done that before
<headius> this is the traditional way we have been doing super from Java code
<enebo[m]> This solution makes sense to me even if it forces some extra overhead
<enebo[m]> What I am now thinking is how important this is
<enebo[m]> I don't like people not getting a fix but I am questioning whether we release this in a week in case we get some other problems
<headius> the main problem with always supering from Set is acquiring the Set class... we don't have an easy reference to it from here
<headius> I guess this is why we do security releases
<enebo[m]> It is clearly critical for the person who reported it but they are also seemingly using an earlier version
<headius> 9.2.20 is still affected in any case but nobody reported it there
<headius> so it has been in the wild for over a month
<headius> perhaps this means people are moving off of 9.2 😀
<enebo[m]> maybe. I guess it also depends on how many people are using concurrent set
<headius> hash hit on Object to acquire Set class
<headius> but no frame
<headius> enebo: so it is up to you... this is a pretty standard way to do super dispatch, I was just overzealous in trying to optimize the frame away
<headius> it works in several other core classes... because many of them can't be extended
<headius> the long term right fix is to pass frame class and method name into the java method
<headius> I am annoyed by this as well but it looks like a showstopper for the reporter
<enebo[m]> yeah I don't really have an opinion on which fix
<enebo[m]> I am more concerned how when we push the fix
<headius> that's what I meant
<enebo[m]> oh maybe I will think about this differently in a couple of hours but I think there is some merit to seeing what else falls out over the next few days
<headius> yeah I agree
<headius> if we merge the fix and push some snapshots this guy could possibly use that but a release on Monday is pretty quick
<enebo[m]> So I think we plan on releasing a point for 9.2/9.3 when I get back from my week off
<enebo[m]> yeah that is a good idea
<enebo[m]> he has reported other problems in the last few months
<enebo[m]> If I remember he is the person who hit the gem push IOError roo
<headius> I commented on the issue
<enebo[m]> great
drbobbeaty has quit [Ping timeout: 252 seconds]
drbobbeaty has joined #jruby
drbobbeaty has quit [Ping timeout: 256 seconds]
drbobbeaty has joined #jruby
subbu has joined #jruby
subbu has quit [Quit: Leaving]
subbu has joined #jruby
subbu has quit [Quit: Leaving]