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 Proxy Method Handling (toString, equals, hashCode) in OS Signal handling #9358

Closed
wants to merge 2 commits into from

Conversation

Saturn225
Copy link
Contributor

@Saturn225 Saturn225 commented Nov 29, 2024

This PR resolves an issue where the Signal.scala implementation caused a stack overflow during signal handling. The issue arises because java.lang.reflect.Proxy was used for signal handling and default methods (toString, equals, hashCode) were not explicitly handled. As a result, Calls to toString on the proxy object led to recursive invocations as reported in the issue and reproducer

This fix draws from established practices for proxy behavior:

Implementing equals, hashCode and toString for Proxies

The fix adheres to Java's expectations and prevented recursion by ensuring these methods are handled directly in the InvocationHandler

/claim #9321
Fixes #9321

Let me know any for clarifications of my solution or any suggestion or feedback to incorporate

Copy link

algora-pbc bot commented Nov 29, 2024

💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe.

@Saturn225
Copy link
Contributor Author

@jdegoes Can have any reviews on this PR?

Copy link
Contributor

@kyri-petrou kyri-petrou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm quite sceptical on whether this indeed a fix to the linked issue. The issue exists only for Windows platforms but this PR doesn't really change anything specific to Windows. Can you please explain how does it solve the issue, as well as screenshots / videos that it indeed fixes the issue when running an application on a Windows machine?

clarzte pushed a commit to clarzte/zio that referenced this pull request Dec 2, 2024
@Saturn225
Copy link
Contributor Author

Saturn225 commented Dec 3, 2024

@kyri-petrou This fix is related of StackOverflow issue recorded in reproducer. I thought from logs this is due to shift in 2.1.12 the Signal Handling from ZIO specific to JAVA Signal Hadler

Exception in thread "SIGINT handler" java.lang.StackOverflowError
	at zio.internal.Signal$SignalHandler$SunMiscSignalHandler.$anonfun$initInvocationHandler$1(Signal.scala:114)
	at jdk.proxy2/jdk.proxy2.$Proxy2.toString(Unknown Source)
	at java.base/java.lang.StringConcatHelper.stringOf(StringConcatHelper.java:453)
	at zio.internal.Signal$SignalHandler$SunMiscSignalHandler.$anonfun$initInvocationHandler$1(Signal.scala:114)
	at jdk.proxy2/jdk.proxy2.$Proxy2.toString(Unknown Source)
	at java.base/java.lang.StringConcatHelper.stringOf(StringConcatHelper.java:453)
	at zio.internal.Signal$SignalHandler$SunMiscSignalHandler.$anonfun$initInvocationHandler$1(Signal.scala:114)
	at jdk.proxy2/jdk.proxy2.$Proxy2.toString(Unknown Source)
	...

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

Successfully merging this pull request may close these issues.

OS Signal handling and fiber interruption
2 participants