• Type: Bug
• Component: core-libs
• Sub-Component: java.lang:reflect
• Affected Version: 8u121

• Priority: P4
• Status: Resolved
• Resolution: Fixed

• Submitted: 2017-02-10
• Updated: 2017-11-29
• Resolved: 2017-02-28

JDK 8	Other
8u141Fixed	openjdk7uFixed

Peter Levart provided the attached reproducer (ProxyRace.java) for the race condition. It is configured to run for 5 seconds and when run on a desktop machine it is very consistent in manifesting the error on the old build. It never manifests it on the patched build. It might make a useful regression test with the proviso that the test may intermittently fail to detect the bug in the unpatched code depending on timing of execution. 15-02-2017 This bug appears to be at the root of a prior JIRA relating to deserialization which was dismissed as relating simply to a reachability problem https://bugs.openjdk.java.net/browse/JDK-8087168 I have a reproducer based on deserialization which led me to identify this fix. The "Not a proxy" issue it reveals is removed by this fix. However, with that fix this second reproducer generates some very arbitrary NullPointerException occurences, whose backtraces show them coming from lines of code which have no associated dereference operations. That's a very strange occurence and suggests that there is indeed something else wrong in the Proxy or Serialization code. 10-02-2017 This issue is not applicable to JDK9 where the proxy caching code has been completely rewritten in order to support modules. 10-02-2017 java.lang.reflect.Proxy uses class WeakCache to retain details of Proxy classes associated with a given class loader. WeakCache uses a two-tier Map to implement the cache. The first tier map employs keys which contain a weak-reference to the classloader. The associated value is also a map. Subkeys indexing this secondary map are built using the first tier key and the collection of interfaces (Class[]) which uniquely identify the required proxy. Values in the secondary map implement a Supplier interface which allows the relevant proxy Class to be produced on demand. WeakCache also maintains a reverse map which maps every secondary map value to Boolean.TRUE. The reverse map is used to implement Proxy.isProxyClass. It allows clients holding a reference to a putative proxy Class to check whether the class really is a proxy. Implementations for values stored in the secondary map are either a CacheValue or a Factory. Initially threads attempting to obtain a proxy Class call Proxy.getProxyClass. Cincurrent calls are dealt with by racing to install a factory in the secondary map (using putIfAbsent) and then call its synchronized get method. n.b. the factory is not entered into the reverse map. Factory.get method creates a proxy, replaces the entry for itself in the secondary map with a CacheValue. The get method for CacheValue is not synchronized. The critical issue is the order of insertion of the CacheValue into the secondary map and reverse map. At present Factory.get calls map.replace to install the CacheValue into the secondary map before calling map.put to install the CacheValue in the reverse map. A thread suspend between these two operations allows some other thread to lookup the proxy class (e.g. by calling Proxy.resolveClass), retrieve a class cl which passes the test cl instanceof Proxy yet fails the check Proxy.isProxyClass(cl). This precise sequence of tests happens when deserializing proxy instances and leads to an InvalidClassException with message "Not a proxy". A simple fix for this issue is to reverse the order of the secondary map replace and the reverse map put. n.b. A test case to manifest this issue and validate the fix has been provided by Peter Levart. 10-02-2017