I was trying to avoid too many changes, but in the end I decided it was
better to refactor some of this code into a new method. So here is the
new webrev:
http://cr.openjdk.java.net/~mullan/webrevs/8075706/webrev.02/
Changes include:
- bootstrapping code moved to new method named loadPolicyProvider
- added Alan's suggestion to fallback to SCL only if CNFE is thrown
- instantiated default policy provider directly instead of using
reflection if property has not been changed or is not set
- addressed Mandy's comment about formatting
- enhanced test to cover more cases
Thanks,
Sean
On 05/04/2015 04:14 PM, Sean Mullan wrote:
On 05/04/2015 10:55 AM, Alan Bateman wrote:
On 01/05/2015 13:48, Sean Mullan wrote:
Please review this change to use the system class loader to locate a
policy provider specified with the policy.provider security property.
The current implementation tries to load the provider using the
extension class loader. This will no longer work when the extension
mechanism is removed, which is proposed as part of JEP 220:
http://openjdk.java.net/jeps/220
webrev: http://cr.openjdk.java.net/~mullan/webrevs/8075706/webrev.01/
The change looks okay but I wonder if there is an opportunity to do some
clean-up.
Yes, definitely, but OTOH it's been mostly working fine for years, and
it's a fragile area, so we need to proceed with care :)
One thing that is a bit icky is the catching of Exception for the case
that the policy class is loaded by boot loader but it can't be
instantiated for some reason. It might be better to replace this with
Class.forName(policy_class, false, null) and only fallback to trying
again with the SCL if CNFE is thrown. That way you could have once place
to instantiate the policy class and log the exception if it fails.
That seems like a good idea. I'll change it, and if I get a good JPRT
run, I'll proceed with that change.
--Sean
