Fix races in publish_subscribe_test.rb and update truffleruby

[eregon]

• See https://github.com/oracle/truffleruby/issues/3321

[byroot]

You angered rubocop, but otherwise LGTM.

[eregon]

Fixed, and the CI passed except for that rubocop complaint: https://github.com/redis/redis-rb/actions/runs/6904372003/job/18784907013?pr=1239

[eregon]

https://github.com/redis/redis-rb/actions/runs/6904417499/job/18785060895?pr=1239

  1) Failure:
TestPublishSubscribe#test_subscribe_from_another_thread [/home/runner/work/redis-rb/redis-rb/test/redis/publish_subscribe_test.rb:332]:
Expected nil to not be nil.

This is unrelated to this PR, it's doing refute_nil thread.join(2).

Thread#join with a duration is basically an anti-pattern, even more so in tests. I'll push a commit removing the duration, time can vary a lot in CI, so a duration is just asking for more transients.

[byroot]

Thread#join with a duration is basically an anti-pattern, even more so in tests.

The reason is to avoid locking forever, if the thread is stuck trying to read on a socket or some other blocking operation, your CI runner hang for long enough that it's just killed and you have 0 debug information.

I'm not against bumping it, but I wouldn't remove it.

[eregon]

If running locally, one would Ctrl+C and then get a stacktrace, so there better without a duration as well.

Yeah, GitHub Actions doesn't send SIGINT on timeout I guess, unfortunate. IMO it should be the job of the test harness to print thread stracktraces if e.g. a test takes longer than a large timeout (e.g. some minutes). Note the failure above gives very little information.

[byroot]

IMO it should be the job of the test harness to print thread stracktraces

Sure, but in the meantime minitest doesn't offer that, so until it does, a generous .join(5) is prefereable to .join.

Note the failure above gives very little information.

They at least give you which test failed, which is a decent start.

[eregon]

BTW there are many other cases of thread.join in the same file, so I'd suggest to try it like this, and if a test is hanging transiently and cannot be reproduced easily locally then either modify that one to e.g. show thread.backtrace if not joined in N seconds, or improve the test harness (or in test helpers maybe) to do that automatically.

[eregon]

As you wish, should I remove the last commit from this PR or keep it?

[byroot]

Please remove it yes.

[eregon]

OK.

They at least give you which test failed, which is a decent start.

That's also shown if the test hangs, because the test name is printed before it starts running.

[eregon]

Removed, and ready to merge.