Skip to content

java 11 compatibility: handle a new exception if a field is inaccessible - #59

Closed
sda399 wants to merge 5 commits into
ehcache:masterfrom
sda399:master
Closed

java 11 compatibility: handle a new exception if a field is inaccessible#59
sda399 wants to merge 5 commits into
ehcache:masterfrom
sda399:master

Conversation

@sda399

@sda399sda399 commented Sep 20, 2019

Copy link
Copy Markdown
  • setAccessible function now throws a new exception
  • catch the new exception class that appeared in java9+ only

Samir Saada
java 11 compatibility: handle a new exception if a field is inaccessible
259aadf
- setAccessible function now throws a new exception
- catch the new exception class that appeared in java9+ only
@sda399

sda399 commented Sep 20, 2019•
edited
Loading

Copy link
Copy Markdown
Author

Hi all,
this is a simple patch to fix a java11 compat I run into.

java.lang.reflect.InaccessibleObjectException: Unable to make field jdk.internal.ref.PhantomCleanable jdk.internal.ref.PhantomCleanable.prev accessible: module java.base does not "opens jdk.internal.ref" to unnamed module @3c153a1
at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:340)
at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:280)
at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:176)
at java.base/java.lang.reflect.Field.setAccessible(Field.java:170)
at org.ehcache.sizeof.ObjectGraphWalker.getAllFields(ObjectGraphWalker.java:275)
at org.ehcache.sizeof.ObjectGraphWalker.getFilteredFields(ObjectGraphWalker.java:234)
at org.ehcache.sizeof.ObjectGraphWalker.walk(ObjectGraphWalker.java:176)
at org.ehcache.sizeof.SizeOf.deepSizeOf(SizeOf.java:75)

henri-tremblay
henri-tremblay reviewed Sep 21, 2019
View reviewed changes
Comment threadsrc/main/java/org/ehcache/sizeof/ObjectGraphWalker.java Outdated
henri-tremblay
henri-tremblay suggested changes Sep 21, 2019
View reviewed changes

@henri-tremblayhenri-tremblay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

== null to fix.

chrisdennis
chrisdennis requested changes Sep 24, 2019
View reviewed changes
Comment threadsrc/main/java/org/ehcache/sizeof/ObjectGraphWalker.java Outdated
Comment threadsrc/main/java/org/ehcache/sizeof/ObjectGraphWalker.java Outdated
@mprusakov

mprusakov commented Sep 18, 2020

Copy link
Copy Markdown
Contributor

@chrisdennis would it be possible to merge this please? The library doesn't really work well with java 11 without this change and as far as i can tell all the issues have been fixed. I'd be more than happy to assist if any more changes are required

henri-tremblay
henri-tremblay reviewed Sep 18, 2020
View reviewed changes
Comment threadsrc/main/java/org/ehcache/sizeof/ObjectGraphWalker.java
} catch (RuntimeException e) {
// new class (jdk9+ only) can be thrown
if ("java.lang.reflect.InaccessibleObjectException".equals(e.getClass().getName())) {
LOG.error("JPMS blocks field access. This prevents Ehcache from accessing "

@henri-tremblayhenri-tremblaySep 18, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should probably be just a warning. Because it might occur a lot and there's nothing we can do about it.

chrisdennis
chrisdennis requested changes Sep 18, 2020
View reviewed changes
Comment threadsrc/main/java/org/ehcache/sizeof/ObjectGraphWalker.java
LOG.error("Security settings prevent Ehcache from accessing the subgraph beneath '{}'" +
" - cache sizes may be underestimated as a result", field, e);
continue;
} catch (RuntimeException e) {

@chrisdennischrisdennisSep 18, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right now this is swallowing thrown RuntimeException instances without logging anything and then adding an inaccessible field to the collection. I think it's going to be simpler (and cleaner) to just:

} catch (RuntimeExceptione) {
LOG.warn("The JVM is preventing Ehcache from accessing the subgraph beneath '{}'" +
" - cache sizes may be underestimated as a result", field, e);
}

@mprusakov

mprusakov commented Sep 18, 2020

Copy link
Copy Markdown
Contributor

Unfortunately i do not have access to the original for had to re-fork and re-raise the pr, available here #61

@chrisdennis

chrisdennis commented Sep 18, 2020

Copy link
Copy Markdown
Member

Replaced with #61

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

Reviewers

@chrisdennischrisdennischrisdennis requested changes
+1 more reviewer
@henri-tremblayhenri-tremblayhenri-tremblay requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sda399@mprusakov@chrisdennis@henri-tremblay