Synchronization layer

[pitr-ch]

link #241

If you agree with the approach I'll merge this PR because of fixes in d66f616 and I'll start migrating all the other classes to this layer in another PR. Event is migrated as an example.

[jdantonio]

I'm sorry for not having an opportunity to provide detailed feedback yet. I'm participating in SpaceApps this weekend and I've been busy preparing. I did look over the PR and I really like the direction you have taken. 👍 Consolidating the helpers into 'engine' is a nice redactor. The fact that it touched so many files is an indication of how necessary that was.

[jdantonio]

We should consider removing this entirely for 0.9. since this message was introduced in 0.8. We definitely do not want it in the 1.0 release.

[jdantonio]

👍 I'll rebase #270 and #271 from master once this is merged and get back to work on those updates ASAP.

Thank you!

[pitr-ch]

Thanks for reviewing. Found one more problem which needs fixing. Please do not merge yet.

[pitr-ch]

The issue I've mentioned is fixed in e78b4a5 but I am experiencing failing JRuby tests (e.g. cyclic_barrier), it happens only when whole suite runs, if executed in isolation it's fine :/ I need to find the reason for that before we can merge.

[jdantonio]

@pitr-ch I'm free this evening. I'll work on it tonight, too. Between the two of us we should be able to figure it out. We're pretty smart dudes. :-)

[jdantonio]

@pitr-ch OK, so apparently I'm not as smart as I think I am. I am able to consistently reproduce the failed tests on JRuby but I have been unable to isolate the upstream tests that are causing the problem. Have you had any luck?

[pitr-ch]

Thanks @jdantonio, did you find out anything? I've also continued digging, so far I have:

• if JRuby compilation is disabled (-J-Djruby.compile.mode=OFF) it works as expected
• if debugging prints are added
  • ns_wait_until reports its return value correctly
  • any print in CyclicBarrier#wait is skipped, it looks like the body of this method is skipped too, it returns in the spec where the barrier's state looks like the body of wait was skipped
• reproducer: only cyclic_barrier_spec where spec breaks the barrier and release all other threads is run 100x until it compiles and starts to fail, on jruby-1.7.19
• I've also tried pure Java version of JavaSynchronizedObject, it fails too.

I am going to sleep on it before deciding what to do next :)

update: I've read your comment after posting this. I am also feeling pretty dumb, after 3 days of intermittent investigation.

[pitr-ch]

It looks like if while true is replaced with loop do at https://github.com/ruby-concurrency/concurrent-ruby/blob/synchronization/lib/concurrent/synchronized_object.rb#L80 it starts to work 🎆

[jdantonio]

Interesting. I'll pull the latest branch and see if it works for me, too. Thanks for keeping at it!

[jdantonio]

@pitr-ch As far as I can tell your latest update fixed it. On my local machine I rebased this branch onto master (where I merged PR #275) and the tests pass on both JRuby and MRI. I haven't pushed that update yet because I wanted to coordinate with you, first.

I'm comfortable rebasing this PR from master then merging it. That will allow us to work on the other PRs. I also plan to revisit the at_exit handlers for the global thread pools and fix the discrepancy you pointed out a couple of weeks ago.

Are you comfortable with us committing this PR?

[pitr-ch]

Thanks. I'll need to fix few TODOs in the code and verify that all the failing tests are known intermittently failing tests. After that, I'll merge it.

@jdantonio please review #272 first, I've already touched the at_exit handlers there.

[jdantonio]

@pitr-ch Can we delete this branch?

[pitr-ch]

@jdantonio done

[jdantonio]

Thank you!