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

Project Loom does not play well with synchronized, so WDYT of adding support for "forbid use of synchronized"? #213

Open
vlsi opened this issue Dec 16, 2022 · 4 comments
Assignees

Comments

@vlsi
Copy link
Contributor

vlsi commented Dec 16, 2022

In pgjdbc, we replace all synchronized with explicit Lock: pgjdbc/pgjdbc#2635
I wonder if I could use forbidden-apis to avoid more synchronized keywords creeping in the future.

@uschindler
Copy link
Member

uschindler commented Dec 16, 2022

Good idea. But this would be an additional "feature" as it requires to not just parse for forbidden methods calls (invoke/invokespecial/invokestatic/getfield/putfield bytecode), but just look at attributes of methods.

  • I have to check how to detect a synchronized block inside bytecode. For method declartion it is just access flag (as far as I remember).
  • Add some setting to enable it globally.
  • Make sure SuppressForbidden works.

@uschindler uschindler self-assigned this Dec 16, 2022
@vlsi
Copy link
Contributor Author

vlsi commented Dec 16, 2022

Well, synchronized(something) would use monitorenter bytecode (e.g. https://asm.ow2.io/javadoc/org/objectweb/asm/Opcodes.html#MONITORENTER )

synchronized methods would indeed be coded as method modifiers.

On top of that, it would probably be great to include Object#wait, Object#notify, and Object#notifyAll into the forbidden set as they make sense only in case synchronized is used (they should be replaced with Condition#await, Condition#signal, Condition#signalAll).

I've no idea if synchronized methods or monitorenter could appear in synthetic code, or in the code generated by Kotlin, Scala, or the other compilers.
However, pgjdbc is Java-only for now, so it would be nice to just forbid plain Java synchronized

@uschindler
Copy link
Member

Kotlin must also use monitor enter or attributes on methods, unless they use locks internally. Groovy at least uses it.

@marinier
Copy link

FWIW, the issue with using synchronized with virtual threads will be fixed in Java 24: https://openjdk.org/jeps/491

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

No branches or pull requests

3 participants