JDK 14 RFR of JDK-8232234: Suppress warnings on non-serializable non-transient instance fields in java.rmi

Tue Oct 15 17:35:40 UTC 2019

Hi Roger,

In rough statistics, there are well over 1,000 serializable classes in 
the JDK. There are closer to a dozen classes that are also 
externalizable (java.io.Externalizable extends java.io.Serializable ).

As such, the new checks have had much more tuning against 
serializable-but-not-externalizable classes compared to externalizable ones.

With any analysis, there is a balance between reporting false positives 
(as in this case due to {read, write}External) with false negatives (not 
reporting on a situation of actual concern).

It would be a small matter of programming to guard the field 
serializability checks with a " ... && notExternalizable()" clause. That 
would eliminate the warning in cases like LiveRef, at the risk of not 
reporting on possible logic errors or omissions.

Clearly the case for these checks is less strong for externalizable than 
serializable-but-not-externalizable ones. On balance, do you think these 
checks should be skipped for externalizable classes?

Cheers,

-Joe

On 10/15/2019 8:28 AM, Roger Riggs wrote:
> Hi Joe,
>
> In the case of UnicastRef, I think the warning on LiveRef ref is too 
> aggresive.
> Since UnicastRef inherits Externalizable and has writeExternal and 
> readExternal
> methods there is no static indication of which fields of the class are 
> serialized or in what form.
> All of the logic for what and how the fields are serialized is in the 
> code of those methods.
>
> What's the rationale for requiring developers to suppress warning on 
> fields that
> are not known to be serialized/externalized.
>
> Roger
>
>
> On 10/15/19 1:12 AM, Joe Darcy wrote:
>> Hello,
>>
>> Please review the changes for serial review now extending to the 
>> java.rmi module:
>>
>>     JDK-8232234: Suppress warnings on non-serializable non-transient 
>> instance fields in java.rmi
>> http://cr.openjdk.java.net/~darcy/8232234.0/
>>
>> Patch below.
>>
>> For 
>> src/java.rmi/share/classes/sun/rmi/server/ActivatableServerRef.java, 
>> the absence of no-arg constructor prevents the Externalizable class 
>> from being deserialized (de-externalized?), but as it hasn't seemed 
>> to cause problems in the 16 years since the file was last modified, I 
>> didn't think it was worthwhile to add a constructor at this point.
>>
>> Thanks,
>>
>> -Joe
>>
>> --- 
>> old/src/java.rmi/share/classes/java/rmi/activation/ActivationGroup.java 
>> 2019-10-14 21:49:08.305824614 -0700
>> +++ 
>> new/src/java.rmi/share/classes/java/rmi/activation/ActivationGroup.java 
>> 2019-10-14 21:49:08.117824614 -0700
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All 
>> rights reserved.
>> + * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All 
>> rights reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -107,6 +107,7 @@
>>      /**
>>       * @serial the group's monitor
>>       */
>> +    @SuppressWarnings("serial") // Not statically typed as Serializable
>>      private ActivationMonitor monitor;
>>
>>      /**
>> --- 
>> old/src/java.rmi/share/classes/java/rmi/activation/ActivationGroupID.java 
>> 2019-10-14 21:49:08.845824614 -0700
>> +++ 
>> new/src/java.rmi/share/classes/java/rmi/activation/ActivationGroupID.java 
>> 2019-10-14 21:49:08.641824614 -0700
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All 
>> rights reserved.
>> + * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All 
>> rights reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -49,6 +49,7 @@
>>      /**
>>       * @serial The group's activation system.
>>       */
>> +    @SuppressWarnings("serial") // Not statically typed as Serializable
>>      private ActivationSystem system;
>>
>>      /**
>> --- 
>> old/src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java 
>> 2019-10-14 21:49:09.333824614 -0700
>> +++ 
>> new/src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java 
>> 2019-10-14 21:49:09.133824614 -0700
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 1996, 2017, Oracle and/or its affiliates. All 
>> rights reserved.
>> + * Copyright (c) 1996, 2019, Oracle and/or its affiliates. All 
>> rights reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -189,12 +189,14 @@
>>      /**
>>       * @serial client-side socket factory (if any)
>>       */
>> +    @SuppressWarnings("serial") // Not statically typed as Serializable
>>      private RMIClientSocketFactory csf = null;
>>
>>      /**
>>       * @serial server-side socket factory (if any) to use when
>>       * exporting object
>>       */
>> +    @SuppressWarnings("serial") // Not statically typed as Serializable
>>      private RMIServerSocketFactory ssf = null;
>>
>>      /* indicate compatibility with JDK 1.1.x version of class */
>> --- 
>> old/src/java.rmi/share/classes/sun/rmi/server/ActivatableServerRef.java 
>> 2019-10-14 21:49:09.837824614 -0700
>> +++ 
>> new/src/java.rmi/share/classes/sun/rmi/server/ActivatableServerRef.java 
>> 2019-10-14 21:49:09.645824614 -0700
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 1997, 2003, Oracle and/or its affiliates. All 
>> rights reserved.
>> + * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All 
>> rights reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -38,6 +38,7 @@
>>   *
>>   * @author Ann Wollrath
>>   */
>> + at SuppressWarnings("serial") // Externalizable class w/o no-arg c'tor
>>  public class ActivatableServerRef extends UnicastServerRef2 {
>>
>>      private static final long serialVersionUID = 2002967993223003793L;
>> --- old/src/java.rmi/share/classes/sun/rmi/server/Activation.java 
>> 2019-10-14 21:49:10.413824614 -0700
>> +++ new/src/java.rmi/share/classes/sun/rmi/server/Activation.java 
>> 2019-10-14 21:49:10.173824614 -0700
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 1997, 2014, Oracle and/or its affiliates. All 
>> rights reserved.
>> + * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All 
>> rights reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -145,9 +145,11 @@
>>      private static boolean debugExec;
>>
>>      /** maps activation id to its respective group id */
>> +    @SuppressWarnings("serial") // Conditionally serializable
>>      private Map<ActivationID,ActivationGroupID> idTable =
>>          new ConcurrentHashMap<>();
>>      /** maps group id to its GroupEntry groups */
>> +    @SuppressWarnings("serial") // Conditionally serializable
>>      private Map<ActivationGroupID,GroupEntry> groupTable =
>>          new ConcurrentHashMap<>();
>>
>> @@ -297,6 +299,7 @@
>>
>>          private static final String NAME = 
>> ActivationSystem.class.getName();
>>          private static final long serialVersionUID = 
>> 4877330021609408794L;
>> +        @SuppressWarnings("serial") // Not statically typed as 
>> Serializable
>>          private ActivationSystem systemStub = null;
>>
>>          SystemRegistryImpl(int port,
>> @@ -498,6 +501,7 @@
>>       * with RegistryImpl.checkAccess().
>>       * The kind of access is retained for an exception if one is 
>> thrown.
>>       */
>> +    @SuppressWarnings("serial") // Externalizable class w/o no-arg 
>> c'tor
>>      static class SameHostOnlyServerRef extends UnicastServerRef {
>>          private static final long serialVersionUID = 1234L;
>>          private String accessKind;      // an exception message
>> @@ -873,7 +877,9 @@
>>          ActivationGroupDesc desc = null;
>>          ActivationGroupID groupID = null;
>>          long incarnation = 0;
>> +        @SuppressWarnings("serial") // Conditionally serializable
>>          Map<ActivationID,ObjectEntry> objects = new HashMap<>();
>> +        @SuppressWarnings("serial") // Conditionally serializable
>>          Set<ActivationID> restartSet = new HashSet<>();
>>
>>          transient ActivationInstantiator group = null;
>> --- 
>> old/src/java.rmi/share/classes/sun/rmi/server/ActivationGroupImpl.java 
>> 2019-10-14 21:49:11.057824614 -0700
>> +++ 
>> new/src/java.rmi/share/classes/sun/rmi/server/ActivationGroupImpl.java 
>> 2019-10-14 21:49:10.841824614 -0700
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All 
>> rights reserved.
>> + * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All 
>> rights reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -69,6 +69,7 @@
>>          new Hashtable<>();
>>      private boolean groupInactive = false;
>>      private final ActivationGroupID groupID;
>> +    @SuppressWarnings("serial")  // Conditionally serializable
>>      private final List<ActivationID> lockedIDs = new ArrayList<>();
>>
>>      /**
>> --- old/src/java.rmi/share/classes/sun/rmi/server/UnicastRef.java 
>> 2019-10-14 21:49:11.701824614 -0700
>> +++ new/src/java.rmi/share/classes/sun/rmi/server/UnicastRef.java 
>> 2019-10-14 21:49:11.445824614 -0700
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 1996, 2017, Oracle and/or its affiliates. All 
>> rights reserved.
>> + * Copyright (c) 1996, 2019, Oracle and/or its affiliates. All 
>> rights reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -67,6 +67,7 @@
>> Boolean.getBoolean("sun.rmi.client.logCalls")));
>>      private static final long serialVersionUID = 8258372400816541186L;
>>
>> +    @SuppressWarnings("serial") // Not statically typed as Serializable
>>      protected LiveRef ref;
>>
>>      /**
>>
>