> On 9 Jun 2016, at 15:18, Roger Riggs <roger.ri...@oracle.com> wrote: > > Hi Mark, > > IIOPInputStream.java: > > - Does adding doPriv overhead to every field access have a noticeable impact > on performance? > If most of the fields are accessible or can easily be checked as being in > the base module, > I could see trying the access directly and catching the security exception > to retry with doPriv.
Maybe something like what is done in sun.security.action.GetPropertyAction.privilegedGetProperty(String) -Chris. > > Roger > > ps. > > There is a newer version of webrev that provides convenient next and previous > links. > > http://hg.openjdk.java.net/code-tools/webrev/raw-file/d26c194772db/webrev.ksh > > > > On 6/9/2016 8:57 AM, Seán Coffey wrote: >> Hey Mark, >> >> Looks fine. A few comments. >> >>> 2291 Class<?> declaredFieldClass = >>> declaredClassField.getType(); >>> 2292 >>> 2293 if (declaredFieldClass == null) { >>> 2294 continue; >>> 2295 } >> I'm not sure if this check is required. Can getType() return null ? >> >> == >> >>> 2833 Field classField; >>> 2834 classField = c >>> 2835 .getDeclaredField(fieldName); >>> 2836 return classField; >> Could we fold that code down to 'return c.getDeclaredField(fieldName);' ? >> >> For the tests : >> == >> ConcurrentHashMapTest.java : >> you might want to remove the old rmiServerProcess.destroy(), >> orbdProcess.destroyForcibly() comments. >> == >> HelloClient.java >> line 157 : s/1000/ONE_SECOND >> >> The use of static ports could be one to watch. If we encounter port issues, >> the test might have to be revisited. >> >> Regards, >> Sean. >> >> On 08/06/2016 23:18, Mark Sheppard wrote: >>> >>> Hi >>> please oblige and review the following changes >>> http://cr.openjdk.java.net/~msheppar/8146975/jdk9/webrev/ >>> >>> http://cr.openjdk.java.net/~msheppar/8146975/jdk9/test/webrev/ >>> >>> which address the issue raised in >>> https://bugs.openjdk.java.net/browse/JDK-8146975 >>> >>> the type checking in inputClassFields and other places failed to fully >>> allowing for >>> the processing of return ValueTypes, and hence the getDeclaredField fails as >>> "application code" exist on the call stack restricting access. This leads >>> to a security exception, >>> which in turn leads to an IllegalArgumentExcetption, the processing of >>> which failed to allow >>> for a null object value in the stream. >>> This has now been rectified, with the getDeclaredField wrapped in a >>> doPrivileged call. >>> >>> regards >>> Mark >> >
