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

No need to synchronise a newly created local RubyArray #2451

Merged
merged 2 commits into from Jan 12, 2015

Conversation

@Who828
Copy link
Contributor

@Who828 Who828 commented Jan 12, 2015

Possibly fixes #2446, we don't need synchronise for a locally created RubyArray .

@headius
Copy link
Member

@headius headius commented Jan 12, 2015

This is fine to remove but we also need to add correct synchronization, either against the thread list or against the whole ThreadGroup.

@headius
Copy link
Member

@headius headius commented Jan 12, 2015

Or rather, to fix #2446 we need the appropriate synchronization. This PR alone does correctly remove useless synchronization on the Array :-)

@Who828
Copy link
Contributor Author

@Who828 Who828 commented Jan 12, 2015

Oops! I did a little bit of digging and I found out that synchronizedSet must be manually synchronised while iterating http://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#synchronizedSet(java.util.Set), so I have made the change.

@headius
Copy link
Member

@headius headius commented Jan 12, 2015

Looks good to me!

headius added a commit that referenced this pull request Jan 12, 2015
No need to synchronise a newly created local RubyArray
@headius headius merged commit b887145 into jruby:master Jan 12, 2015
1 check passed
1 check passed
@enebo
continuous-integration/travis-ci The Travis CI build passed
Details
headius added a commit that referenced this pull request Jan 12, 2015
No need to synchronise a newly created local RubyArray
@headius
Copy link
Member

@headius headius commented Jan 12, 2015

Also applied to 1.7 in d99bfd9.

@headius headius self-assigned this Jan 12, 2015
@Who828 Who828 deleted the Who828:sync_thread_group_fix branch Jan 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants