RFR: JDK-8146975 - NullPointerException in IIOPInputStream.inputClassFields

[Mark Sheppard]

Thanks Sean, I'll incorporate your amendments

wrt static ports, it is a problems, but for these test one that is 
unavoidable

ConcurrentHashMapTest incurs intermittent failures due to port already 
in use issue.
I'll make a change for a bug against this, in the near future,  to 
overcome and lessen the probability of
its occurrence.

regards
Mark

On 09/06/2016 13:57, 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
>