05:34 <asp_> Hello. I have come across a weird issue in JRuby involving Mutexes and ConditionVariables.

05:35 <asp_> Minimal example is here: https://gist.github.com/asppsa/c74b654a006475c44729cc57cd3fd86e

05:35 <asp_> On MRI this code seems to work fine, but on jruby 9.2.8.0 it works for a bit and then locks up

05:38 <headius[m]> Hello

05:39 <asp_> Hi

05:41 <headius[m]> I don't see anything wrong at a glance. She it is hung, press Ctrl+\ or use jstack command and gist that output

05:41 <headius[m]> She it = when it

05:41 <asp_> Ok, will do

05:43 <asp_> Right, added the Ctrl+\ output as a comment to the gist

05:48 <asp_> Actually, I can make the example even more minimal by using only a single conditionvariable

05:48 <headius[m]> The threads are both in wait on different locks 🤔

05:51 <asp_> OK, well I kinda expected that it was going to be my code that was at fault... would it help if I tested on git master and/or 9.1?

05:53 <headius[m]> Yeah worth a try...I'm on mobile at the moment so a bit limited

05:54 <headius[m]> Strange

05:56 <headius[m]> The impl of wait seems wrong

05:57 <headius[m]> It releases the lock and then waits on a semaphore unrelated to the lock, which means signal doesn't do anything

05:57 <headius[m]> So main thread rolls right through signal and waits too

05:57 <headius[m]> Kinda shocking that this is broken

05:59 <headius[m]> https://github.com/jruby/jruby/commit/bea9ad42f62e24a3963e135f2138c75a716f7790

05:59 <headius[m]> Recently broken. Must be a big gap in tests for this

06:00 <headius[m]> Try going back a few versions and if you find it works open an issue with your gist

06:00 <asp_> ok sure

06:00 <asp_> I can confirm the issue is definitely not present in the last 9.1 release

06:00 <headius[m]> A spec for spec/ruby/... based on your example would be great too, fine to do as a PR for us

06:01 <headius[m]> Looks like I tried to fix interruptibility and broke signalling, but nothing tests this

06:01 <headius[m]> Yeah file it for sure then

06:01 <headius[m]> Will fix for 9.2.9

06:02 <asp_> K, will do

06:35 <asp_> https://github.com/jruby/jruby/issues/5863

06:35 <asp_> Wasn't sure about the title - hope that's all right. I'll have a go at writing a spec.

06:48 <headius[m]> Looks good, thanks!

06:48 <headius[m]> Should be working in 9.2.7 or .6

06:49 <asp_> Yep, it's working in 9.2.7

06:53 <headius[m]> 👍

12:51 <rdubya[m]> headius: saw your bug report on Symbol#to_s not returning the same string object (https://bugs.ruby-lang.org/issues/16150), we use that relatively frequently in our app, would it make sense/be possible to add a jruby flag that could be set to have it always return frozen strings (maybe it applies to all "primitives" like you mentioned in the ticket)

12:51 <rdubya[m]> Just thought maybe that would be a way to optionally get the functionality in before the full language changes get hammered out

12:52 <headius[m]> That's a good idea. We could test everything. I'm still traveling but it might be a trivial PR to submit if you want to try it

12:52 <headius[m]> PR doesn't have to make it optional at first and we can see what fails

12:53 <headius[m]> Just need another field in RubySymbol to cache the frozen RubyString

12:53 <headius[m]> You could try throwing your app at the branch too

12:54 <rdubya[m]> Cool, yeah I'll see if I can figure it out

12:56 <headius[m]> I don't know why someone didn't think of this earlier. All that String allocation

12:56 <headius[m]> Credit to schneems for the inspiration

12:56 <rdubya[m]> I guess I just assumed that it was already happening lol

12:57 <headius[m]> It didn't occur to me until his talk and then is was duh

12:57 <headius[m]> We do reuse the same ByteList but it's still a waste of an object

12:59 <rdubya[m]> we make pretty extensive use of HashWithIndifferentAccess and its doing this constantly: https://github.com/rails/rails/blob/master/activesupport/lib/active_support/hash_with_indifferent_access.rb#L366

13:00 <headius[m]> Yeah that's where schneems found it

13:00 <headius[m]> Or something related

13:00 <headius[m]> Oh actually it wasn't indifferent so the client code has to always to_s but same deal

13:01 <headius[m]> His patch was very localized...this broader change could have a big impact

13:01 <headius[m]> Also nil, true, false

13:02 <rdubya[m]> I wonder if it would make sense to have a patch on indifferent access that stores a hash of cached strings, would mean a double hash lookup every reference though, so would probably slow things down

13:06 <headius[m]> That's essentially what schneems did in another location

13:07 <headius[m]> I think it was for respond_to? Against a model...lazily caches the sym to string

13:07 <rdubya[m]> cool, yeah could see that getting hot too