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

Weird synchrinization blocks #765

Closed
kofemann opened this issue Aug 29, 2023 · 3 comments · Fixed by #774
Closed

Weird synchrinization blocks #765

kofemann opened this issue Aug 29, 2023 · 3 comments · Fixed by #774

Comments

@kofemann
Copy link

By looking at the code I noticed some weird synchronization blocks, like:

    public String toString(int indentFactor) throws JSONException {
        StringWriter w = new StringWriter();
        synchronized (w.getBuffer()) {
            return this.write(w, indentFactor, 0).toString();
        }
    }

https://github.com/stleary/JSON-java/blob/master/src/main/java/org/json/JSONObject.java#L2573

Obviously, synchronizing on a newly created object makes no sense.

@johnjaylward
Copy link
Contributor

Yes, all instances of that seem to be pretty useless. I don't see how any other threads would be able to write to the writers and require synchronization.

If anything, I would have expected synchronization locks in the more generic write methods that take a Writer as a parameter.

@stleary
Copy link
Owner

stleary commented Sep 3, 2023

This has been in the code for over 10 years. But agree it seems unnecessary. No objection if someone wants to remove these instances of synchronization.

@stleary
Copy link
Owner

stleary commented Oct 1, 2023

Fixed in #774

@stleary stleary closed this as completed Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants