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

fix unnecssary synchronized issue for NativeJavaPackage.getPkgProperty: #601

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

qxo
Copy link

@qxo qxo commented Sep 25, 2019

not synchronized for get , only synchronized for put

@gbrail
Copy link
Collaborator

gbrail commented Sep 27, 2019

Thanks for looking at this!

I'm not sure I agree that this is unnecessary synchronization. It looks to me that after the change, we are testing the contents of "negativeCache" outside the "synchronized" block, and in that case I don't believe we can guarantee that that object will behave correctly.

@qxo
Copy link
Author

qxo commented Sep 29, 2019

move "negativeCache" check code to synchronized block ?

@gbrail
Copy link
Collaborator

gbrail commented Oct 3, 2019

Yes -- I think that the check of "negativeCache" still needs to be inside the "synchronized" block if we need this class to be thread-safe.

On the other hand, regular Rhino objects are no longer thread-safe by default unless configured that way, so we could make an argument that we should do the same thing here.

Can you give us a little more detail on what problem you're trying to fix?

not synchronized for get , only synchronized for put
@qxo
Copy link
Author

qxo commented Oct 8, 2019

My user-case was many thread block on org.mozilla.javascript.NativeJavaPackage.getPkgProperty(NativeJavaPackage.java:113) from javacore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants