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

Java 9: InaccessibleObjectException: private final byte[] java.lang.String.value #169

Closed
don-vip opened this issue May 18, 2017 · 16 comments
Labels
Milestone

Comments

@don-vip
Copy link
Contributor

don-vip commented May 18, 2017

What steps will reproduce the problem?

With Java 9 run any test causing EqualsVerifier attempting to call Field.setAccessible on java.lang.String.value.

What error message or stack trace does EqualsVerifier give?

InaccessibleObjectException: Unable to make field private final byte[] java.lang.String.value accessible: module java.base does not "opens java.lang" to unnamed module @2a5ca609

junit.framework.AssertionFailedError: InaccessibleObjectException: Unable to make field private final byte[] java.lang.String.value accessible: module java.base does not "opens java.lang" to unnamed module @2a5ca609
	at nl.jqno.equalsverifier.EqualsVerifier.handleError(EqualsVerifier.java:355)
	at nl.jqno.equalsverifier.EqualsVerifier.verify(EqualsVerifier.java:344)
	at org.openstreetmap.josm.data.osm.FilterTest.testEqualsContract(FilterTest.java:246)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at org.openstreetmap.josm.testutils.JOSMTestRules$TimeoutThread.run(JOSMTestRules.java:387)

What did you expect?

No error.

Which version of EqualsVerifier are you using?

2.2.1

Please provide any additional information below.

The following entry should be added to Manifest to fix this error:

Add-Opens: java.base/java.lang

@don-vip
Copy link
Contributor Author

don-vip commented May 18, 2017

The Manifest is missing from jar published on Maven Central?

@jqno
Copy link
Owner

jqno commented May 19, 2017

Hm, you're right, the manifest disappeared somewhere around version 2.0. I never noticed. I'll look into it.

In the mean time, I'm a little surprised at the error you're reporting. EqualsVerifier has built-in prefabvalues for String, so I don't think it should be doing reflection on it. Clearly it's doing so anyway. Can you tell me a little bit more about the code that triggers this issue? Are you testing a simple POJO with a String field in it, or is there something else going on?

Also, did you give me the full stacktrace, or is there a Caused by section as well? In that case, I'd be very interested to see that as well.

Finally, I wonder if setting Add-Opens to java.base/java.lang is enough; I'm doing reflection on a whole bunch of JVM classes. I clearly need to read up on what's new in Java 9. Do you happen to know a good resource on this particular subject? 😄

@jqno jqno added the accepted label May 19, 2017
@don-vip
Copy link
Contributor Author

don-vip commented May 19, 2017

It's not yet fully clear about what's going on in Java 9. There many recent changes not yet described in JEP 261, partly linked to the recent "No" vote of the JCP regarding Jigsaw. But as Oracle has only a few days to propose a revised draft, the documentation will be quickly updated !
We have described in JOSM all the changes we have noticed because they impacted us directly, with this ticket: https://josm.openstreetmap.de/ticket/11924
In a more complete way, the best resource is probably the JDK9 draft release notes: http://jdk.java.net/9/release-notes

Back to the point. I didn't submit a code sample because we get this error in a large number of tests (24 exactly), see https://josm.openstreetmap.de/jenkins/job/Java-EarlyAccess-JOSM/lastCompletedBuild/testReport/

If I look at the first one (FilterTest):

    @Test
    public void testEqualsContract() {
        EqualsVerifier.forClass(FilterPreferenceEntry.class).usingGetClass()
            .suppress(Warning.NONFINAL_FIELDS)
            .verify();
    }

Here is the FilterPreferenceEntry class:

import org.openstreetmap.josm.data.Preferences.pref;
import org.openstreetmap.josm.data.Preferences.writeExplicitly;

public class Filter extends SearchSetting {

    public static class FilterPreferenceEntry {
        @writeExplicitly
        @pref public String version = "1";

        @pref public String text;

        /**
         * Mode selector which defines how a filter is combined with the previous one:<ul>
         * <li>replace: replace selection</li>
         * <li>add: add to selection</li>
         * <li>remove: remove from selection</li>
         * <li>in_selection: find in selection</li>
         * </ul>
         * @see SearchMode
         */
        @writeExplicitly
        @pref public String mode = "add";

        @pref public boolean case_sensitive;

        @pref public boolean regex_search;

        @pref public boolean mapCSS_search;

        /**
         * Enabled status.
         * @see Filter#enable
         */
        @writeExplicitly
        @pref public boolean enable = true;

        /**
         * If this option is activated, the chosen objects are completely hidden.
         * Otherwise they are disabled and shown in a shade of gray.
         * @see Filter#hiding
         */
        @writeExplicitly
        @pref public boolean hiding;

        /**
         * Normally, the specified objects are hidden and the rest is shown.
         * If this option is activated, only the specified objects are shown and the rest is hidden.
         * @see Filter#inverted
         */
        @writeExplicitly
        @pref public boolean inverted;

        @Override
        public int hashCode() {
            return Objects.hash(case_sensitive, enable, hiding, inverted, mapCSS_search, mode, regex_search, text, version);
        }

        @Override
        public boolean equals(Object obj) {
            if (this == obj)
                return true;
            if (obj == null || getClass() != obj.getClass())
                return false;
            FilterPreferenceEntry other = (FilterPreferenceEntry) obj;
            return case_sensitive == other.case_sensitive
                    && enable == other.enable
                    && hiding == other.hiding
                    && inverted == other.inverted
                    && mapCSS_search == other.mapCSS_search
                    && regex_search == other.regex_search
                    && Objects.equals(mode, other.mode)
                    && Objects.equals(text, other.text)
                    && Objects.equals(version, other.version);
        }
    }
}

And the related annotations (maybe this is the root cause?):

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

public class Preferences {
    /**
     * Annotation used for converting objects to String Maps and vice versa.
     * Indicates that a certain field should be considered in the conversion process. Otherwise it is ignored.
     *
     * @see #serializeStruct(java.lang.Object, java.lang.Class)
     * @see #deserializeStruct(java.util.Map, java.lang.Class)
     */
    @Retention(RetentionPolicy.RUNTIME) // keep annotation at runtime
    public @interface pref { }

    /**
     * Annotation used for converting objects to String Maps.
     * Indicates that a certain field should be written to the map, even if the value is the same as the default value.
     *
     * @see #serializeStruct(java.lang.Object, java.lang.Class)
     */
    @Retention(RetentionPolicy.RUNTIME) // keep annotation at runtime
    public @interface writeExplicitly { }
}

@don-vip
Copy link
Contributor Author

don-vip commented May 21, 2017

Damn, my suggestion does not work. "Add-Opens" directive is only taken into account for the jar files executed through "java -jar" command. Jars on the classpath are ignored. So we must find why the reflection occurs and prevent it. I'll try to find the original stacktrace.

@don-vip don-vip changed the title Java 9: add "Add-Opens: java.base/java.lang" to Manifest Java 9: InaccessibleObjectException: private final byte[] java.lang.String.value May 21, 2017
@don-vip
Copy link
Contributor Author

don-vip commented May 21, 2017

The stacktrace with equalsVerifier 2.2.2 and jdk-9+170 is:

Thread [main] (Suspended (exception java.lang.reflect.InaccessibleObjectException))	
	java.lang.reflect.Field(java.lang.reflect.AccessibleObject).checkCanSetAccessible(java.lang.Class<?>, java.lang.Class<?>, boolean) line: 341	
	java.lang.reflect.Field(java.lang.reflect.AccessibleObject).checkCanSetAccessible(java.lang.Class<?>, java.lang.Class<?>) line: 281	
	java.lang.reflect.Field.checkCanSetAccessible(java.lang.Class<?>) line: 175	
	java.lang.reflect.Field.setAccessible(boolean) line: 169	
	nl.jqno.equalsverifier.internal.FieldAccessor.modify(nl.jqno.equalsverifier.internal.FieldAccessor$FieldModifier, boolean) line: 194	
	nl.jqno.equalsverifier.internal.FieldAccessor.copyTo(java.lang.Object) line: 169	
	nl.jqno.equalsverifier.internal.ObjectAccessor<T>.copyInto(S) line: 133	
	nl.jqno.equalsverifier.internal.ObjectAccessor<T>.copy() line: 98	
	nl.jqno.equalsverifier.FieldsChecker$ReflexivityFieldCheck.checkValueReflexivity(nl.jqno.equalsverifier.internal.FieldAccessor, nl.jqno.equalsverifier.internal.FieldAccessor) line: 367	
	nl.jqno.equalsverifier.FieldsChecker$ReflexivityFieldCheck.execute(nl.jqno.equalsverifier.internal.FieldAccessor, nl.jqno.equalsverifier.internal.FieldAccessor) line: 336	
	nl.jqno.equalsverifier.FieldInspector<T>.check(nl.jqno.equalsverifier.FieldInspector$FieldCheck) line: 41	
	nl.jqno.equalsverifier.FieldsChecker<T>.check() line: 58	
	nl.jqno.equalsverifier.EqualsVerifier<T>.verifyWithExamples() line: 405	
	nl.jqno.equalsverifier.EqualsVerifier<T>.performVerification() line: 367	
	nl.jqno.equalsverifier.EqualsVerifier<T>.verify() line: 338	
	org.openstreetmap.josm.data.osm.FilterTest.testEqualsContract() line: 246

@don-vip
Copy link
Contributor Author

don-vip commented May 21, 2017

And this is enough to trigger the exception:

    public static class FilterPreferenceEntryStub {
        public String version = "1";

        @Override
        public int hashCode() {
            return Objects.hash(version);
        }

        @Override
        public boolean equals(Object obj) {
            if (this == obj)
                return true;
            if (obj == null || getClass() != obj.getClass())
                return false;
            FilterPreferenceEntryStub other = (FilterPreferenceEntryStub) obj;
            return Objects.equals(version, other.version);
        }
    }

@don-vip
Copy link
Contributor Author

don-vip commented May 21, 2017

I see there are prefab values "one", "two" for String, but that not prevents the code to perform a shallow copy by reflection. Maybe you should also store prefab "copy constructors" for well-known classes, in this case public String(String original).

@jqno
Copy link
Owner

jqno commented May 21, 2017

Thanks for the detailed information!

I'm currently working on a release that I'd like to finish up first. When that's done, I'll download a recent JDK9 and see if I can reproduce and fix this issue. While I'm at it, I'll see if I can make some progress on your other issue #152 as well.

I have a couple of busy weeks coming up though, so I can't make any promises about when I can do all this. I will keep you posted.

@don-vip
Copy link
Contributor Author

don-vip commented May 21, 2017

ok thanks! If you don't work on this issue this week, you'll probably have a great change in JDK9 that will prevent you to reproduce the bug. To achieve JCP consensus Oracle is currently stepping back on the most controversial breaking changes, such as the strong encapsulation that prevents unnamed modules to perform this kind of reflection, see http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-May/012673.html

So to reproduce the issue (as the code will probably work by then, but not fixing it would only delay its correction to JDK10) you will need to add the command line argument --illegal-access=deny

@jqno
Copy link
Owner

jqno commented May 21, 2017

OK, thanks for the heads-up!

floscher pushed a commit to floscher/josm that referenced this issue May 23, 2017
floscher pushed a commit to floscher/josm that referenced this issue May 23, 2017
@jqno
Copy link
Owner

jqno commented May 30, 2017

I've reproduced the issue. I'll just summarise all of my findings, much of which you probably already know, just for later reference :).

The following command-line options must be given to the JVM to use EqualsVerifier:

--add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.base/java.time=ALL-UNNAMED --add-opens=java.naming/javax.naming=ALL-UNNAMED

(Not all of these are necessary in every project, but they are if you want to build EqualsVerifier itself.)

Since EqualsVerifier runs as a unit test, you need to give these paramters to Maven's surefire plugin, like so:

    <build>
        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-surefire-plugin</artifactId>
                <configuration>
                    <argLine>--add-opens java.base/java.lang=ALL-UNNAMED</argLine>
                </configuration>
            </plugin>
        </plugins>
    </build>

I agree with you that the proper fix would probably be to have some kind of "prefab" copy constructor for known types. This approach has two drawbacks though. First, it probably wouldn't work for types from third-party modules. By its nature, EqualsVerifier is a reflection-heavy tool, after all. Second, it would be a lot of work to implement.

As you mentioned, Oracle may be reducing the scope of breaking changes in the near future. Therefore, I think I'll wait until the dust settles a little, rather than spend a lot of energy right now to fix things that may not be broken in the final release of Java 9.

Once the scope of this issue becomes clearer, I will start working on a proper fix. In the mean time, the workaround would be to add the necessary --add-opens lines to your Maven script.

@jqno jqno added the pending label May 30, 2017
@jqno jqno added this to the Java 9 milestone May 30, 2017
simon04 pushed a commit to JOSM/josm that referenced this issue Jun 2, 2017
simon04 pushed a commit to JOSM/josm that referenced this issue Jun 2, 2017
@jqno
Copy link
Owner

jqno commented Aug 6, 2017

I've installed Java 9+181, and as we suspected, they've indeed made this a non-breaking change. Updating to the most recent Java 9 should solve the problem.

However, I'm (of course) still getting "illegal reflective access" warnings when I compile EqualsVerifier on Java 9. I've opened #172 to deal with the warnings, and I'm closing this issue.

@jqno jqno closed this as completed Aug 6, 2017
@erikvasilyan
Copy link

Getting same error on Java 21+, is there any update?

@jqno
Copy link
Owner

jqno commented Sep 30, 2024

This issue is 7 years old, a lot has changed since then. If you get this error, it's very likely due to a different reason. Please open a new ticket so we can discuss it

@erikvasilyan
Copy link

erikvasilyan commented Sep 30, 2024

@jqno the issue was that i added Embedded annotation on a String type field, solved just by removing the annotation, but curious that i got the same error as here

@jqno
Copy link
Owner

jqno commented Sep 30, 2024

As I said, same error message but very different cause. I'd be happy to help you but please open a new ticket for it, so the other participants in this ticket don't get spammed.

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

No branches or pull requests

3 participants