dopplergange has quit [Ping timeout: 260 seconds]
dopplergange has joined #jruby
ur5us has joined #jruby
sagax has quit [Ping timeout: 260 seconds]
ur5us has quit [Ping timeout: 260 seconds]
ur5us has joined #jruby
Osho_ has quit [Remote host closed the connection]
ur5us has quit [Ping timeout: 260 seconds]
sagax has joined #jruby
_whitelogger has joined #jruby
_whitelogger has joined #jruby
drbobbeaty has quit [Ping timeout: 258 seconds]
drbobbeaty has joined #jruby
olleolleolle has quit [Ping timeout: 240 seconds]
olleolleolle has joined #jruby
subbu is now known as subbu|away
ur5us has joined #jruby
victori_ has joined #jruby
victori has quit [Ping timeout: 240 seconds]
subbu|away is now known as subbu
<headius[m]> aargh
<headius[m]> checking errno immediately after waitpid call in Process.waitpid and it is indeed 11 (EAGAIN)
<headius[m]> also confirmed step debugging that this is not using the pthread_kill logic
<headius[m]> enebo: mrnoname reproduced on his Linux env
<headius[m]> he has some wacky package setup but not nix
<headius[m]> try this command line on your env
<headius[m]> bin/jruby -Xbacktrace.style=full -J-Djnr.ffi.asm.enabled=false -J-Djruby.compile.mode=OFF -Xnative.pthread_kill=false -rjruby -e "pid = spawn 'date'; Process.waitpid pid"
<headius[m]> `bin/jruby -Xbacktrace.style=full -J-Djnr.ffi.asm.enabled=false -J-Djruby.compile.mode=OFF -Xnative.pthread_kill=false -rjruby -e "pid = spawn 'date'; Process.waitpid pid"`
<enebo[m]> yay
<headius[m]> starting to suspect this is a false error
<headius[m]> give that command line a try and I will look into glibc code
<enebo[m]> This works for me
<enebo[m]> Although I am using master
<headius[m]> can you try 9.2.14
<headius[m]> working theory at this point is that some waitpid logic, either in libc or in our binding, is not being sanitary about errno
<enebo[m]> yeah almost built
<headius[m]> so the waitpid call returns the pid as though it was successful, but errno is EAGAIN for some unrelated reason
<enebo[m]> still works
<headius[m]> fascinating
<headius[m]> what is your glibc version?
<headius[m]> actually you could check 9.2.13
<enebo[m]> 2.2.5 I think (I used nm)
<headius[m]> I have not actually confirmed this on anything but 9.2.13 on fzakaria image
<headius[m]> mrnoname says it happens on musl too
<enebo[m]> still works
<headius[m]> that would start to point back at jnr-ffi not being tidy
<headius[m]> ugh but if it works for you that is so confusing
<headius[m]> bottom line at this point is that we should be checking for -1 retval in any case, which would fix it, but I want to know why errno is not 0 after a successful waitpid
<enebo[m]> yeah
<headius[m]> waitpid code falls into kernel very quickly and I can't find my way through that
<enebo[m]> afk a little bit
<headius[m]> let me know what kernel version when you are back
<enebo[m]> headius: 5.8.13-200.fc32.x86_64
<headius[m]> ok
<headius[m]> I pinged Andrew Haley about this because he pinged me a couple months ago about some weird rogue EAGAIN during JVM process logic
<headius[m]> he thought it was our fault but in the end he found some kernel module was causing it
<headius[m]> maybe this case will sound familiar
subbu is now known as subbu|away
<headius[m]> enebo: I have pushed https://github.com/jruby/jruby/pull/6489 to fix waitpid and wait to check for -1 return value
<enebo[m]> headius: hmm so that fixes it
<headius[m]> enebo: yeah "fixes"
<headius[m]> it doesn't explain why errno is nonzero after a successful waitpid
<enebo[m]> I approve of that PR as much as one can knowing it is not the root cause :)
<enebo[m]> but it is better than no fix
<headius[m]> I can find no commonality between your system and fzakaria and mrnoname machines
<headius[m]> mrnoname even tested on musl libc and it does the same thing
<headius[m]> but it works for you
<headius[m]> it is baffling
<enebo[m]> so something with non-generated code works on my system and I presume most but not on some
<enebo[m]> have you tried master with new jnr-constants on tha image?
subbu|away is now known as subbu
<headius[m]> not on the image but on mrnonane
<headius[m]> he tested a fresh build of master and it still fails for him
<enebo[m]> ok
<headius[m]> 🤷‍♂️
<enebo[m]> so may as well paper over this and when it happens somewhere else we will have two things to draw off of
<headius[m]> Hopefully Andrew has some thoughts because I am kinda out of ideas
<enebo[m]> some times it just takes another report
<headius[m]> it could still be our issue but I would expect it to happen consistently then
<enebo[m]> yeah it is definitely possible it is glibc or something
<headius[m]> can you try running it a bunch more times just for my sanity?
<headius[m]> it is intermittent and might depend on whether the subprocess has completed by the time we call waitpid
<enebo[m]> 12 runs on 9.2.13.0 all work
<headius[m]> I have only had it pass when it should fail a few times
<enebo[m]> HAHAHA loop it and see if it endlessly fails
<enebo[m]> I would expect it would
<headius[m]> I did for a while and it pretty much always failed
<headius[m]> and then a random pass running from command line, like either it was faster or slower to get to the process
<headius[m]> I dunno it is bugging me and I am going to punt for now with the PR
<enebo[m]> I mean within same Ruby process
<headius[m]> yeah could do
<enebo[m]> -e 'loop {...}'
<enebo[m]> If it is intermittent then that would be something to ponder
<enebo[m]> although that would even more imply some signal interrupt issue
enebo has left #jruby [#jruby]
<headius[m]> only first fails
<headius[m]> but why
<headius[m]> forcing a sleep after spawn does not make it happen
<headius[m]> why only the first time
<enebo[m]> HAHAH wow
<headius[m]> spawn itself should have triggered loading of jnr-posix internals
<headius[m]> waitpid just goes straight into libc
<enebo[m]> so wait no sleep but just a loop and only first fails
<headius[m]> consistently
<enebo[m]> ok well that is significant :P
<headius[m]> starting to think it is something in the first run of the jnr-ffi invoker leaving an errno dangling
<headius[m]> and then subsequent calls go straight through without that logic
<enebo[m]> your guess is mine...something not fully inited
<enebo[m]> that makes sense too
<enebo[m]> but I thought you determined that errno was reset to 0 right before waitpid?
<headius[m]> I even forced it
<headius[m]> that didn't help
<enebo[m]> but doing it without forcing it in a loop somehow works
<headius[m]> yes
<headius[m]> I will try stepping all the way into jnr-ffi
<headius[m]> well that told me nothing... it seems to take the same path each time, with the exception of preparing the invoker the first time
<headius[m]> nothing in the invoker prep appears to involve native calls
<headius[m]> but the first time leaves EAGAIN in errno and the second time it is zero
ur5us has quit [Ping timeout: 264 seconds]
<headius[m]> tell me the answer
<headius[m]> ok, I'm changing gears
<headius[m]> enebo: hey I noticed this while tab sweeping if you want to do some fun kwargs fixing... https://github.com/jruby/jruby/issues/6452
<headius[m]> I only ping you because you have worked on kwargs coercion logic in the past
<enebo[m]> wow. what is proper behavior
<headius[m]> I just added a comment that I don't know
<headius[m]> I am assuming we are coercing too much since only this one form causes the problem, but I have not confirmed what CRuby does
<headius[m]> in any case we have differing logic along this path compared to other forms
<enebo[m]> hmm
<enebo[m]> This is specifically an issue with JI I think
<enebo[m]> mri26 -e 'def foo(a, **b); p a, b; end; foo(a:1)'
<headius[m]> note this specific super form is required to trigger it on JRuby
<headius[m]> I tried several other paths and they don't cause it
<enebo[m]> ok so zsuper is transforming this to something and the hashmap is somehow becoming a kwargs
<enebo[m]> but I am surprised we do not see this without JI
<headius[m]> yeah
<headius[m]> well most stuff won't coerce to hash
<headius[m]> you can do it without JI by defining a to_h on some object and try that on CRuby
<enebo[m]> jruby ../snippets/hash3.rb
<enebo[m]> NilClass
<headius[m]> it is only doing this if you super into a *a, **b super method with a trailing argument that can coerce to Hash
<enebo[m]> that is master
<headius[m]> hmm
<enebo[m]> run that
<headius[m]> wat
<enebo[m]> There is some issue here I think but adding contents shows it still converts to Hash but not because of kwargs
<headius[m]> I am confused now
<enebo[m]> your original snippet also confuses me
<headius[m]> I wonder if this broke temporarily on master
<enebo[m]> arr[0] should be nil
<headius[m]> I cannot repro it anywhere now
<enebo[m]> ok well you're welcome
<headius[m]> well cone
<headius[m]> well done cone
<enebo[m]> yeah so arr would return the hash backwhen args is called
<headius[m]> right
<enebo[m]> I guess the expectation is a HashMap would not become a Hash
<headius[m]> right it should not
<enebo[m]> So that does still happen
<enebo[m]> but I do not think it has anything to do with kwargs
<enebo[m]> oh but it does
<enebo[m]> it is hashmap with kwargs removed
<enebo[m]> ok so it is converting **kwargs is probably forcing a coercion of the single argiment in the splat
<enebo[m]> which is becoming a hash as a result but then kwargs is just triggering whatever does that
<enebo[m]> so this is valid
<enebo[m]> I am a bit concerned this might not end up fixable
<enebo[m]> since this is happening in what I am guessing is an IR helper but perhaps it is if we are doing something weird with zsuper?
<headius[m]> wait so it is doing something it shouldn't?
<headius[m]> I am trying to show that CRuby doesn't do it the same but output matches
<enebo[m]> if you remove kwargs from lower blah it shows HashMap and not Hash
<enebo[m]> So kwargs will change the HashMap into a Hash (e.g. coerce it from Java -> Ruby)
<headius[m]> on master
<enebo[m]> yeah
<enebo[m]> hmm maybe...let me verify
<enebo[m]> I switched back to master but realized I have an error in a stash I just popped
<enebo[m]> grr 9.2.13.0
<enebo[m]> recompiling
<headius[m]> this seems the same on 9.2 HEAD, master HEAD, and CRuby 2.6.5
<headius[m]> args contains the uncoerced object
<enebo[m]> I am seeing Hash
<headius[m]> assuming this is a valid test
<headius[m]> gah really?
<headius[m]> jruby -v
<enebo[m]> I am using your snippet
<enebo[m]> it is master
<enebo[m]> well I did add an element to the hash just so it was not empty
<headius[m]> I was on a branch but it was merged recently... checking master proper now
<enebo[m]> If I remove **kwargs it works
<headius[m]> try the gist I just posted
<enebo[m]> [#<#<Class:0x2ea6137>:0x4c40b76e>]
<enebo[m]> {}
<headius[m]> ok so maybe it is specific to JI?
<enebo[m]> no I think you needed to_hash and not to_h
<enebo[m]> although MRI does work that way too
<headius[m]> wtf
<headius[m]> ahh sure
<enebo[m]> So to_hash I believe is being called due to presence of **kwargs...let's verify that
<headius[m]> with to hash we both do coerce
<enebo[m]> yeah if I remove **kwargs from MRI run no to_hash is called
<headius[m]> ok so I am getting a picture
<headius[m]> 9.2 looks the same
<enebo[m]> So we pass in a HashMap and because it has to_hash and **kwargs exists it does it...pretty weird but I believe this is how it works
<headius[m]> so this may actually be correct logic but HashMap should perhaps not to_hash
<enebo[m]> your Class.new helps a lot in proving that
<headius[m]> which would be in keeping with other coercion methods... to_str, to_ary etc are supposed to only work for a natural string or array I think
<headius[m]> people don't follow that but maybe JI should
<enebo[m]> So this references rspec...did something break?
<headius[m]> yeah something in rspec expectations introduced a **kwargs format and it caused the incoming value to get converted
<headius[m]> it was a recent change to better support kwargs in expectations
<enebo[m]> but this will happen to any code which happens to pass a single element which has to_hash on it
<headius[m]> so it caused our specs to fail because the HashMap started getting converted
<headius[m]> it was "fun" to diagnose
<headius[m]> yeah so this isn't a bug
<headius[m]> but I am leaning toward JI HashMap not doing to_hash
<enebo[m]> so either a) we should remove to_hash if it is so common but then I think we will get other reports b) tell them this will likely cause confusion for them as well
<headius[m]> that is the new debate I guess
<headius[m]> yeah like you say I dunno if this has a fix
<headius[m]> this is the behavior
<enebo[m]> Another thought would be to do to_hash and return self
<headius[m]> so if they are going to have **kwargs in this call path they will risk converting expected values that define to_hash
<enebo[m]> Ultimately that would probably duck type properly but would not reflect properly
<headius[m]> that seems like a bigger risk... if anything to_hash should return a natural hash
<enebo[m]> well I would argue anyone who depends on to_hash would be surprised to see it go away too
<headius[m]> HashMap can still have a to_h
<headius[m]> yeah but is that a thing?
<enebo[m]> but I don't know the use case of this
<headius[m]> most people call to_h
<headius[m]> internals may call to_hash though
<enebo[m]> yeah I really cannot comprehend the scope of this
<enebo[m]> it is so implicit
<headius[m]> or this could be argued as an rspec bug and they should not be causing coercible objects to follow a path where they will coerce
<headius[m]> right
<enebo[m]> I also wonder how much people even know to_hash is happening too
<headius[m]> you and me
<headius[m]> and the folks wrestling with 2.7/3.0 kwargs behavior
<enebo[m]> yeah I can say from a pure-Ruby perspective it could happen but will anyone notice
<headius[m]> maybe I should file an rspec issue and tag some folks that are kwargs adjacent
<enebo[m]> some object which is not Hash but will make one but once made will stop working in the spec
<enebo[m]> In cases like that it would be someone inappropriately defining something which is not really a hash but then by that logic is anything a hash which is something more than a hash?
<headius[m]> this implicit trailing arg logic goes away in 3.0 it seems
<headius[m]> deprecated in 2.7
<headius[m]> (which I agree now we should just skip and make 9.4 be 3.0)
<headius[m]> or JRuby 10k
<enebo[m]> yeah 2.7 having transitional behavior which is neither 3 not 2.6 seems not worth it
<headius[m]> I will file an rspec issue... at least get more input on this
<headius[m]> and my vote is for removing HashMap.to_hash in 9.3
<enebo[m]> well it will be interesting to see if anything even stops working in our test suite
<headius[m]> true
<headius[m]> will do it as a PR
<headius[m]> would not change in 9.2.x in any case
<enebo[m]> At some level the need to make something a real Ruby Hash should be muted by duck-typing on the Ruby side
<enebo[m]> OTOH if you make HashMap -> Hash on Ruby and then pass it back to Java then it will stay a Ruby Hash but act like a HashMap with per element coercion
<enebo[m]> by not doing that it will stay unconverted on Java side but on Ruby side it will do per element coercion all the time
<enebo[m]> So logically I think we are fine but from a performance notion people may notice hash in Ruby slow down in a case
<enebo[m]> The workaround is not difficult but it is an observable change maybe for someone...or maybe not :)
<enebo[m]> What actually calls to_hash though :)
<headius[m]> yeah not much
subbu is now known as subbu|away
<enebo[m]> 9.3 is significant and I guess we can see
<enebo[m]> I don't know if the rspec usage is enough to convince me we are doing anything wrong but to_hash is pretty toxic in kwargs now
<headius[m]> the problem may go away once we move to 3.0 and it does not coerce the trailing positional arg, if indeed that is the new behavior
<enebo[m]> headius: to_hash not to_h
<enebo[m]> your code snippet will not show it
<headius[m]> ah thanks, old snippit
<headius[m]> I will update original issue and propose we remove to_hash and see what kares says
<enebo[m]> good idea
<headius[m]> I will try to find some other gnarly bug for you
<headius[m]> enebo: rspec bug
<headius[m]> at least according to jeremyevans it should be using ruby2_keywords here rather than the patch they came up with
<headius[m]> I provided a spec that should pass, so I think it is in their court now
<headius[m]> there are spec failures we would have to debate
subbu|away is now known as subbu