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 bug in Java OA constructor causing warning in finalizer #2955

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

externl
Copy link
Member

@externl externl commented Oct 22, 2024

Ensure _state is set to StateDestroyed otherwise we sometimes get a warning from the finalizer. Saw this warning when running tests in a slower container.

- Config: ws,compress,ipv6,serialize,mx
testing load properties from UTF-8 path... ok
testing using Ice.Config with multiple config files... ok
testing configuration file escapes... ok
testing ice properties with set default values...ok
testing ice properties with unset default values...ok
testing that getting an unknown ice property throws an exception...ok
testing that setting an unknown ice property throws an exception...ok
testing that creating an object adapter with unknown properties throws an exception...ok
testing that creating a proxy with unknown properties throws an exception...ok
-! 10/22/24 18:37:52:772 warning: Finalizer: object adapter `FooOA' has not been deactivated

@externl externl added the java label Oct 22, 2024
@externl externl added this to the 3.8.0 milestone Oct 22, 2024
@externl externl changed the title Make finalizer happy Fix bug in Java OA constructor causing warning in finalizer Oct 22, 2024
Copy link
Member

@pepone pepone left a comment

Choose a reason for hiding this comment

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

Looks good, but we should remove Java finalize which was deprecated in Java 9. For a future PR.

@@ -82,7 +82,7 @@ spotless {
}

// Run spotlessApply before compiling Java code
compileJava.dependsOn 'spotlessApply'
// compileJava.dependsOn 'spotlessApply'
Copy link
Member

Choose a reason for hiding this comment

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

Why is this commented out?

Copy link
Member Author

Choose a reason for hiding this comment

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

We shouldn't be doing this. Otherwise we'll never know if the formatting is bad.

Copy link
Member

Choose a reason for hiding this comment

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

then let's remove it.

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

Successfully merging this pull request may close these issues.

3 participants