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

#45 Use Unsafe instantiator on JDK9 #46

Closed
wants to merge 2 commits into from
Closed

#45 Use Unsafe instantiator on JDK9 #46

wants to merge 2 commits into from

Conversation

ropalka
Copy link

@ropalka ropalka commented Oct 28, 2016

Proposal solution for issue #45

@ropalka
Copy link
Author

ropalka commented Oct 28, 2016

When this issue is resolved it would be great to have org.objenesis:objenesis:2.5 released :)

@henri-tremblay
Copy link
Contributor

Thanks. I should be able to have a look and release in about a week

// times slower. So I prefer to use this one
return new SunReflectionFactoryInstantiator<T>(type);
if (PlatformDescription.VM_VERSION.startsWith("9")) {
// UnsafeFactoryInstantiator doesn't work on JDK9 anymore

Choose a reason for hiding this comment

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

Isn't this comment contradictory? 'UnsafeFactoryInstantiator doesn't work in JDK9, so let's use it!'

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed. Is it Unsafe or ReflectionFactory that doesn't work?

Copy link
Author

Choose a reason for hiding this comment

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

Ouch, copy-paste error :( Of course comment should be:
// SunReflectionFactoryInstantiator doesn't work on JDK9 anymore

@ropalka
Copy link
Author

ropalka commented Nov 1, 2016

The method will be resuscitated so false alarm, see:
https://bugs.openjdk.java.net/browse/JDK-8168980

@henri-tremblay
Copy link
Contributor

Funny. Let's consider this unneeded then. Thanks for the alert, the patch, the investigation and the follow-up

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

3 participants