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

Caller sensitive #509

Merged
merged 2 commits into from Aug 4, 2019

Conversation

@Thrameos
Copy link
Contributor

commented Jul 14, 2019

This pull request is working on dealing with the problem with caller sensitive methods. This should remove limitations with forName and the known limitations with Database. This pull request is predicated on previous ones and thus needs prior pulls that it depends on reviewed first.

This first pull is to test with multiple systems. Once it is complete, a review will be requested.

Fixes #508

@Thrameos Thrameos self-assigned this Jul 14, 2019

@Thrameos Thrameos added the bug label Jul 14, 2019

@Thrameos Thrameos added this to the JPype 0.7.1 milestone Jul 14, 2019

@Thrameos Thrameos requested a review from marscher Jul 14, 2019

@Thrameos

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2019

@marscher I believe this one may be ready to go. We are nearing the point that we should cut a 0.7.1 release candidate. We have a few critical bug fixes (wrong exception string on str(), segfault on null array slices) as well as a pretty good list of improvements (annotations, jedi support, pickle, generics in JClass, proxy roundtrip, and caller sensitive). The devel branch is currently at a stall point as I have too many conflicting pull requests to proceed. If you do not have time, I can try to work around it but I will likely have to start merging all these pulls into my local master which tends to get complex if these pull requests require changes.

This particular pull request is targeted towards fixing JDK 12 in which a bug in the JDK made reflection from JNI fail. During the process of resolving that I was able to identify the issues with the previous JPype DatabaseManager limitation and remove it. I am sorry if the pull request is a little unfocused as a lot of minor things either depended on the task, or were detected in during the debugging process.

Items touched

  • A new caller pattern was created to create a dummy stackframe which prevents many JDK calls that used the class of the caller to fail. Thus ends one of the long standing JPype limitation.
  • JDK 9+ triggers the caller pattern on the basis of "CallerSensitive" annotation which a private to the jdk. It took a while to get that pattern down.
  • JDK 1.7/1.8 trigger the pattern for just the three classes known to be buggy.
  • The forName customizer is now been removed as unnecessary.
  • A full test bench for the new caller pattern is developed for JDK 9+. JDK 1.7/1.8 are still supported but they are both clinging to life by a thread. JDK 1.7 is binary format is already triggering "will be removed shortly" and JDK 1.8 binaries are starting to get harder to obtain. I may start using Amazon Corretto as the test platform for 1.8 so that it stays alive another year.
  • Cygwin appveyor patterns got modified to python3.7. For some reason setuptools on cygwin were not being found properly.
  • OSX was enabled as a required platform on travis now that JDK 12 works.
  • The usual spacing corrections were hit, mostly because an upgrade to netbeans 11 caused the formatter patterns to bust. I try to limit the damage, but it is a continuing problem.
  • Pattern to fix Steves name (again) was applied. Not sure what tool does this damage, but as long as the C++ files do not contain a marker for UTF-8 I am afraid this one is going to keep reoccurring.
  • The getJVMVersion pattern had to be revised as the method we were using to get version stopped working in JDK 9 (I have no explanation why they removed it and added a new mechanism, but that is what happened.)
  • documentation was updated to remove the limitation
  • documentation on use of gdb was backported from the development version.
  • unused java_lang_throwable symbol was removed.
  • a minor improvement to the unexpected exception reporting in imports was added.
  • I removed the dynamic_cast to get primitive types in C++ as it was being a pain. This is somewhat geared towards 0.8 where it may be required.
  • bootstrap loader now used loadClass rather than findClass so that it can fall back on the system classloader.
  • a bunch of debugging traces were added as part of tracking down the bug, not required, but do not hurt anything.
  • A pass was done to remove warnings on VC++ while trying to track down weird segfault on that platform.
  • I needed to divide the test harness by versions some more as we have patterns that are generic to all. Others that require lambdas from jdk 8 and then more that require JDK 9+. This can all fall away once we drop support for JDK 8 and earlier. The ant pattern for handling this is a pain in the ass as they didn't add a way to check versions in any meaningful way until 1.10.
  • The tests for calling private methods appears to have been flawed and did not set the accessibility prior to calling. Not sure how it was ever working like that.

Again sorry for dropping mega pull requests. I will be online for about another two weeks before I drop out of sight for a very offline vacation.

@Thrameos

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2019

Fixes #446

@marscher

This comment has been minimized.

Copy link
Collaborator

commented Aug 1, 2019

Indeed a big PR, sorry for the popped up merge conflicts... Since this new feature depends on JDK12, I would prefer that it is added to the test matrix prior this merge. Thank you!

@Thrameos

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2019

I will freshen the PR. Java 12 was added to the test matrix.

Actually, it was already in the matrix. The test of OSX was failing on this issue. With this PR, OSX is now part of the required matrix. I can add a second explicit Java 12 test if you think it is required when I return from vacation.

@Thrameos Thrameos force-pushed the Thrameos:caller_sensitive branch from 1eb1743 to 2440b0a Aug 1, 2019

@Thrameos

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2019

Should be ready for review now.

@marscher marscher merged commit bf6f8b6 into jpype-project:master Aug 4, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Thrameos Thrameos deleted the Thrameos:caller_sensitive branch Aug 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.