Please review the updated webrevs. * Fixed Modules.gmk for order of modules:
http://cr.openjdk.java.net/~sundar/8154192/top/webrev.01/ * From quick reading of j.u.ServiceLoader: AccessControlContext is captured in ServiceLoader constructor & used for iteration (RestrictedIterator). So, ScriptEngineManager calling only ServiceLoader constructor inside doPrivileged block seems fine. Also, I turned ProviderTest javax.script API test to use security manager - this tests loads a dummy script engine from a jar file in classpath. This test passes with security manager on. http://cr.openjdk.java.net/~sundar/8154192/jdk/webrev.01/ Yes, we can still revisit AllPermission for java.scripting module in a future change. Thanks, -Sundar On 5/17/2016 5:54 PM, Alan Bateman wrote: > On 17/05/2016 13:04, Sundararajan Athijegannathan wrote: >> Please review fix for https://bugs.openjdk.java.net/browse/JDK-8154192 >> >> java.scripting module is assigned to platform class loader (instead of >> boot loader). And java.scripting module is given AllPermission >> [previously it had AllPermission implicitly because of being boot loader >> code] >> >> jdk repo: >> >> http://cr.openjdk.java.net/~sundar/8154192/jdk/webrev.00/ >> >> top level repo: >> >> http://cr.openjdk.java.net/~sundar/8154192/top/webrev.00/ >> >> > Would it be possible to keep the PLATFORM_MODULES sorted? Otherwise it > looks okay as a first patch. I think we also need to understand why > this module needs AllPermission. From a quick look then it creates the > SL with all permissions but the iteration will be restricted by > whatever is on the stack so I just wonder if it works as intended. > > -Alan
