<enebo[m]>
This is a head scratcher. I know this is on windows and windows is weird with IO already but why would JIT cause this?
<headius>
oh I glanced at it but did not realize it was only happening in jit
<headius>
yup I have no idwa
<headius>
idea
<enebo[m]>
I marked it for 9.3.9.0 since it is a logstash bug
<headius>
I need to get a windows VM up and going again on new machine
<headius>
aha I was not running right specs... Fiber#transfer is in stdlib 'fiber' technically
<headius>
enebo: I can think of no reason why jit would cause that issue... nothing there should be compiled in and it's all done via method calls and constant lookups
<headius>
need to set a breakpoint in StringIO.new at the point it is called from jitted code and see what's up
<headius>
woot I think I have a repro
<headius>
combination of some weird Fiber#resume that is supposed to error (but doesn't) and trying to transfer from the root fiber... seems like some fiber data gets corrupted
<enebo[m]>
headius: I asked a question...why the ENV["test"] = ""
<headius>
Yeah I don't know what that is either and it shouldn't affect anything
<headius>
Are you able to reproduce this?
<enebo[m]>
I need to try it still
<enebo[m]>
I am just confused out of the gate why JIT would cause this
<enebo[m]>
I am not surprised something broke on windows involving IO but the fact JIT causes it makes no sense to me
<headius>
Nor to me
<enebo[m]>
heh...ENV["test"] = "" is the issue...now to figure out why
<headius>
Wut
<headius>
Is it resetting the external encoding somehow?
<enebo[m]>
but only when JIT :)
<headius>
So in that case the jit would compile the whole script, embedding the encodings of those strings, and if that's the old encoding and ENV is somehow altering the global external encoding, that might cause it to reset
<headius>
Oh but the sequence is wrong
<headius>
The updated external encoding happens after that line
<headius>
🤷♂️
<enebo[m]>
yeah fun one
<headius>
Check that the default external encoding has actually been updated after that line
<enebo[m]>
yeah I am just tracing through Encoding.default_external =
<enebo[m]>
This would change it to whatever is UTF_8
<headius>
Yeah I still don't have a good theory. Even if ENV does mess with the external encoding, the next line should reset it
<enebo[m]>
Ok a new clue
<enebo[m]>
I make "" into a string with forced encoding of UTF-8 and it works
<enebo[m]>
heh...progress but still no clue
<headius>
I don't see anything in ENV logic that would mess with default_external
<enebo[m]>
If I move the ENV to the top it does not change the behavior
<enebo[m]>
It is sort of like if ENV is used the default_external is cached in the JIT
<headius>
enebo: some day I'll learn to test old versions first... this hanging transfer spec also hands on 9.3
<headius>
likely never worked quite right but was hard erroring due to missing features when running under 3.0 mode
<headius>
s/hands/hangs/
<headius>
the problem is a bit tricky... if I am getting this right, main thread resumes fiber1, which then resumes the main fiber... does not detect it is in the process of resuming so it goes ahead and transfers control back... but now the main fiber looks like it is supposed to return to fiber1, which is dead, and a subsequent transfer from main fiber to a new fiber hangs waiting for that dead fiber to pick it up
<headius>
it is supposed to error when fiber1 tries to resume main fiber but because main fiber does not have anyone waiting on it, it looks like it is resumable
<headius>
so it corrupts the main fiber
<headius>
basically incorrectly says "this fiber is being waited on by fiber1, so when it transfers, that's the fiber you should return to"
<headius>
may be simple enough to just always error if trying to resume root fiber from a child fiber, since there's no way you could be doing that unless main fiber were already resuming
<enebo[m]>
headius: heh. yeah re not testing older versions
<headius>
enebo: I pushed a fix for this fiber thing that just hard errors if a fiber tries to resume the root fiber
<headius>
that spec now passes and does not muck up the root fiber's state, so the subsequent transfer spec also runs to completion and passes
<headius>
may need an overhaul of this in the shorter term but that's a bigger job... I feel like I finally understand what transfer is (pass control to the target fiber, but have it return to the fiber waiting on me)
<headius>
enebo: could that stringio bug be modifying some global blank string to have that encoding?
<headius>
JIT mode will cache a single instance of ByteList for all strings of the same content in a given file
<headius>
strDup dups the RubyString object but shares the ByteList
<headius>
it may be using a global blank ByteList
<headius>
we probably should have a ByteList subtype or sibling type that is fully immutable for this and frozen string cases, so there's no chance to mutate it accidentally
<enebo[m]>
OMG
<headius>
JIT uses a global static blank bytelist for empty strings
<enebo[m]>
This makes sense...let me try a non-empty string but this string bank maybe is broken?
<headius>
it does mark it shared but getByteList does not unshare so we end up modifying the global blank bytelist
<headius>
so strDup.getByteList just uses the same shared bytelist again
<headius>
and then we set default Locale Encoding into it
<headius>
if that line I linked above is modified to unshare or explicitly dups the bytelist it will be fixed I bet
<headius>
so yeah we need an immutable bytelist type, which would also potentially optimize frozen strings better too
<headius>
supertype bytelist, subtypes mutable and immutable
<headius>
only bimorphic so even older hotspot should still be able to optimize it
<enebo[m]>
I will remove sharing
<headius>
it should only require making all methods call through getters to get ByteList innards, and then we error if setters are used on immutable one
<enebo[m]>
immutability would have prevented this so that is a good idea
<enebo[m]>
but not a today thing
<headius>
heh no
<enebo[m]>
this not sharing on env values is not really a big deal
<enebo[m]>
if you are constantly setting env then you have at least two problems
<enebo[m]>
hmm it may be used for a bit more but I will test this
<headius>
did a quick experiment in refactoring toward a mutable bytelist and it's probably a day's work to split into mutable and immutable
<headius>
main problem is we have a lot of places that `new ByteList` that would need to go through a factory method to get mutable or immutable appropriately
<headius>
or we can do the weak hack of just introducing ImmutableByteList that overrides setters to error rather than having final fields
<headius>
we lose some benefit of truly final fields but it would catch mutation bugs
<enebo[m]>
download bellsoft liberica
<enebo[m]>
my windows machine has ideas
<enebo[m]>
and those ideas are slow and confusing
<headius>
ha
<headius>
not just a simple refactoring... we arraycopy directly into the array and stuff so each method needs to be reviewed
<headius>
we'd need to go through setter and mutator methods for any mutation so that a subtype can error
<headius>
so yeah it is doable but a project
<enebo[m]>
headius: the fix for today is going to just be str.getByteList().shallowDup() right?
<enebo[m]>
we can still share the backing array of whatever byte[] we are making
<headius>
that should be enough since we only mutate the encoding
<headius>
if we get around to this ByteList rework, you would do dupWithEncoding and have more fluent-style mechanisms for getting a new ByteList from an old one that knows about mutability
<enebo[m]>
My windows box is just not loading JRuby no matter what I try. I don't want to spend 10x the time loading the IDE when I can just fix this on the linux side (I will edit with vi to test though)
<headius>
not loading JRuby?
<headius>
like IntelliJ not loading the project?
<enebo[m]>
ultimately I feel we should consider encoding to be part of sharing
<headius>
well the bigger issue is that ByteList has no knowledge of its shared status
<headius>
this all plays into ByteList being too dumb and all the sharing and code range logic living outside of it
<enebo[m]>
yeah I see
<enebo[m]>
ok well I find the concept of bytelist as something to be redone. It is {bytes, index, size, encoding} but it is clear in how we use it that this tuple is not really enough without ripping all the values out of it
<enebo[m]>
or at least we have had hundreds of bugs from this not quite enough and no abstraction
<headius>
yeah this is a classic bug
<enebo[m]>
yep. It tends to be 'begin' but this is the same sort of issue
<headius>
we need to bake frozen/immutable into ByteList all the way down
<enebo[m]>
yeah
<enebo[m]>
we will get that done when we port over to Kotlin
<headius>
you freeze a string, it flips to a frozen ByteList... if you dup that string for mutation it creates a new mutable ByteList, but if you dup it into a new frozen string it can share
<headius>
hah
<headius>
yeah once we hire a team of interns to do all the work for us
<enebo[m]>
I have wondered how much a language with immutability builtin can eliminate checks in its compiled code
<enebo[m]>
Rust eliminates a ton so I am joking about Kotlin but it does know about immutable values
<enebo[m]>
interesting non of that code is even being hit
<enebo[m]>
That has to be the base problem but that specific code is not being hit by the ENV snippet
<enebo[m]>
Hmm no @JRubyMethod for op_aset until RubyHash
<headius>
it is overridden
<headius>
doesn't matter if it is @JRubyMethod if it overrides the op_aset from RubyHash
<enebo[m]>
yeah
<enebo[m]>
It is not being called
<enebo[m]>
so why would that be?
<headius>
op_aset is overridden by StringOnlyRubyHash, are you saying that is not being called?
<enebo[m]>
I have a print in it and even in newString and nothing is printing
<headius>
that calls case_aware_op_aset which calls newName which does the encoding juggling
<enebo[m]>
I have prints in all of those
<headius>
on Windows?
<enebo[m]>
on windows yeah
<enebo[m]>
I am using vi since I cannot get idea to load the project but I can see maven compiling the file I am changing
<headius>
you sure it'
<enebo[m]>
I can also still see the error go away if I don't jit
<enebo[m]>
no
<headius>
it's running the right one?
<headius>
I don't see how it could not be calling this
<enebo[m]>
:)
<enebo[m]>
Maybe I am calling installed one
<enebo[m]>
I will double check
<headius>
[]= is op_aset, overridden in that env hash
<enebo[m]>
it wouldn't be the firs time
<enebo[m]>
yeah
<enebo[m]>
I mean I know that
<headius>
yeah
<headius>
I suspect pebkac
<headius>
otherwise I have no idea
<enebo[m]>
haha. I switched about 30 tries beforehand
<enebo[m]>
I hate programming
<headius>
hahah
<enebo[m]>
I did a new command line and just typed it because I am quick
<enebo[m]>
then just hit muscle memory
<headius>
it happens... just last week I could not get a debug breakpoint to fire because I was debugging the wrong codebase
<enebo[m]>
yeah
<enebo[m]>
This reminds me that in something else I was writing I needed to use the word content and all I could write is context
<enebo[m]>
Every time I would try and write the word I would write context
<enebo[m]>
ok well shallowDup fixes it :)
<headius>
several of these WIP specs are basically things that always failed but now there's a new version of the spec for 3.x
<enebo[m]>
yeah I have fixed a few where I know it was broken in 9.3 but it I also fixed 3-4 other problems and those were not broken on 9.3
<enebo[m]>
So I should make smaller commits
<enebo[m]>
headius: so why only windows?
<headius>
it may affect unix too but the default filesystem, locale, and internal encodings are usually all UTF-8 so we don't see it
<enebo[m]>
aha
<headius>
I could not reproduce on unix but I'm not sure how to force the locale encoding to be something different
<enebo[m]>
yeah you are right. This is a problem everywhere but it happens to only be windows which is not already UTF-8
<headius>
I am quickly implementing Thread.ignore_deadlock to do nothing... should that be a verbose or non-verbose warning?
<enebo[m]>
ah I see you answered this already. I was going to comment on this but you did it
<headius>
it is a weird feature
<enebo[m]>
hmm
<enebo[m]>
headius: verbose obviously is a simple answer but it depends on how much someone depends on this feature to do something
<headius>
I don't see a way to implement deadlock detection without overhauling locks and doing a lot of work at those boundaries
<headius>
which is why JDK only does it when you pull a thread dump
<headius>
our default is really "ignore" since we don't fast fail when you walk into a deadlock
<enebo[m]>
look for "ignore_deadlock" on github
<enebo[m]>
If a super common gem does it then people will complain
<headius>
3.0 feature, I doubt it is used in the wild much yet
<enebo[m]>
I guess start with verbose and we can promote it if anyone ever notices
<enebo[m]>
but if this does end up some common boilerplate in a library then people will complain that they endlessly see this warning on boot
<headius>
nearly all uses are in tests for this feature
<headius>
there are a couple sets in the wild but all seem to be setting to false
<headius>
three instances in the wild that are not tests or specs for the feature
<enebo[m]>
anyways this works for the double stuff
<enebo[m]>
I will throw it into a PR
<headius>
yeah fix runWithGetsLoop too because that does the compile as well
<headius>
it probably shouldn't duplicate this logic but it does
<enebo[m]>
ok
<headius>
and it catches Throwable... yuck
<enebo[m]>
haha I was just going to point that out
<headius>
so I am done for today.. 3.0 remaining issues are almost all related to scheduler or ractor, with a few little things here and there and the big one being the prepend/include module stuff
<headius>
I suspect we are down to that and a couple other key items and in good shape
<enebo[m]>
refinements issue somewhere which could even be the include/prepend thing
<headius>
Yeah so that and greening up those library suites and then we can do a review of what remains failing
<headius>
I feel like we got several folks already running off of snapshots