Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deadlock in Concurrent::Array's on non-MRI #627

Open
eregon opened this issue Feb 2, 2017 · 4 comments
Open

Deadlock in Concurrent::Array's on non-MRI #627

eregon opened this issue Feb 2, 2017 · 4 comments
Labels
bug A bug in the library or documentation. looking-for-contributor We are looking for a contributor to help with this issue. medium-priority Should be done soon.

Comments

@eregon
Copy link
Collaborator

eregon commented Feb 2, 2017

  • concurrent-ruby version: 1.0.4
  • concurrent-ruby-ext installed: no
  • concurrent-ruby-edge used: no

Take the following code: https://gist.github.com/eregon/323890bfff539f8a33f66d8b2f02cc99
Of course, its purpose is to create a deadlock but it means any method taking a block on Array can cause a deadlock as long as:

  • 2+ threads use 2+ Concurrent::Array by calling a method taking a block
  • inside the blocks, the threads call any method taking on another Concurrent::Array

This seems not such a rare scenario, as it is frequent to call methods in a block that do not only involve the current Array.

The deadlock does not happen on MRI, as it uses a single global lock and concurrent-ruby just subclasses ::Array. It does happen on all other implementations though, like JRuby, Rubinius and TruffleRuby.

The documentation says:

A thread-safe subclass of Array.
This version locks against the object itself for every method call,
ensuring only one thread can be reading or writing at a time.
This includes iteration methods like #each.

So indeed this might imply the deadlock above, but it's not clear.
ensuring only one thread can be reading or writing at a time is also inaccurate on MRI for methods taking a block, as they release the GIL and might switch to another Thread in the middle of e.g. #each.

The same apply for Hash.

Is this behavior intended? Should this be fixed?
Should it behave the same on the different implementations?

@eregon
Copy link
Collaborator Author

eregon commented Feb 2, 2017

Related to #594

@pitr-ch pitr-ch added the bug A bug in the library or documentation. label Feb 12, 2017
@pitr-ch
Copy link
Member

pitr-ch commented Feb 12, 2017

I think this is a bug. 1) it should behave the same on all implementations. 2) it's a good practice not to execute user code when a lock is taken on a data structure, in this case to execute the each block while the block is taken

@thedarkone would you have time to work on this?

@pitr-ch pitr-ch added this to the 1.1.0 milestone Feb 12, 2017
@pitr-ch pitr-ch modified the milestones: 1.1.1, 1.1.0 Apr 9, 2017
@GustavoCaso
Copy link
Contributor

Hi @pitr-ch.
I'm a Ruby developer and I want to get more comfortable working with threads so I decided that the best way is by helping and understanding concurrent-ruby. I'm not very experienced with Threads is there any issue that is a good starting point?

@pitr-ch
Copy link
Member

pitr-ch commented Nov 18, 2017

@GustavoCaso Thanks you very much for you interest. I'll welcome your help. Perhaps you could have a look at #670, it would give you opportunity to read the documentation for the given c-r parts and try it for yourself a with a replacement service you need to find (it can be anything not just finance) for fixing the doc.

@pitr-ch pitr-ch modified the milestones: 1.1.1, 1.1.0 Feb 21, 2018
@pitr-ch pitr-ch added the minor label Jun 18, 2018
@pitr-ch pitr-ch added medium-priority Should be done soon. looking-for-contributor We are looking for a contributor to help with this issue. and removed looking-for-contributor labels Jun 29, 2018
@pitr-ch pitr-ch removed this from the 1.1.0 milestone Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug in the library or documentation. looking-for-contributor We are looking for a contributor to help with this issue. medium-priority Should be done soon.
Projects
None yet
Development

No branches or pull requests

3 participants