JDK-8176192 : Incorrect usage of Iterator in Java 8 In com.sun.jndi.ldap.EventSupport.removeNamingListener
  • Type: Bug
  • Component: core-libs
  • Sub-Component: javax.naming
  • Affected Version: 8,9
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2017-02-23
  • Updated: 2019-01-14
  • Resolved: 2017-06-15
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 10 JDK 8
10 b13Fixed 8u192Fixed
Description
FULL PRODUCT VERSION :


A DESCRIPTION OF THE PROBLEM :
Class: com.sun.jndi.ldap.EventSupport

IntelliJ decomplier code below code below: 

In jdk1.7.0_80 in rt.jar 

    synchronized void removeNamingListener(NamingListener var1) {
        Enumeration var2 = this.notifiers.elements();

        while(var2.hasMoreElements()) {
            NamingEventNotifier var3 = (NamingEventNotifier)var2.nextElement();
            if(var3 != null) {
                var3.removeNamingListener(var1);
                if(!var3.hasNamingListeners()) {
                    var3.stop();
                    this.notifiers.remove(var3.info); -> Works fine, since it���s Enumeration and not iterator 
                }
            }
        }

        if(this.unsolicited != null) {
            this.unsolicited.removeElement(var1);
        }

    }



jdk1.8.0_112 in rt.jar 

    synchronized void removeNamingListener(NamingListener var1) {
        Iterator var2 = this.notifiers.values().iterator();

        while(var2.hasNext()) {
            NamingEventNotifier var3 = (NamingEventNotifier)var2.next();
            if(var3 != null) {
                var3.removeNamingListener(var1);
                if(!var3.hasNamingListeners()) {
                    var3.stop();
                    this.notifiers.remove(var3.info); -> Clearly an incorrect usage of iterator.. This throws a concurrent modification exception 
                }
            }
        }

        if(this.unsolicited != null) {
            this.unsolicited.removeElement(var1);
        }

    }



REGRESSION.  Last worked in version 7u80

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
Remove listener without 
ACTUAL -
Not removing listener 

ERROR MESSAGES/STACK TRACES THAT OCCUR :
java.util.ConcurrentModificationException: null
	at java.util.Hashtable$Enumerator.next(Hashtable.java:1378)
	at com.sun.jndi.ldap.EventSupport.removeNamingListener(EventSupport.java:211)
	at com.sun.jndi.ldap.LdapCtx.removeNamingListener(LdapCtx.java:3445)


REPRODUCIBILITY :
This bug can be reproduced always.

CUSTOMER SUBMITTED WORKAROUND :
Not use the remove listener metod 


Comments
This was introduced by the warnings cleanup work, in this changeset for JDK-7072353. Most of this changeset was to add generics so as to mitigate unchecked and rawtypes warnings. However, one hunk replaced an Enumeration with an enhanced-for loop, which internally uses an Iterator. The relevant hunk of the changeset is below: changeset: 4475:18329abcdb7c user: jjg date: Wed Aug 10 13:44:58 2011 -0700 description: 7072353: JNDI libraries do not build with javac -Xlint:all -Werror Reviewed-by: xuelei Contributed-by: alexandre.boulgakov@oracle.com diff -r 7676670d1e97 -r 18329abcdb7c src/share/classes/com/sun/jndi/ldap/EventSupport.java --- a/src/share/classes/com/sun/jndi/ldap/EventSupport.java Wed Aug 10 16:23:56 2011 -0400 +++ b/src/share/classes/com/sun/jndi/ldap/EventSupport.java Wed Aug 10 13:44:58 2011 -0700 @@ -207,15 +204,11 @@ * Removes <tt>l</tt> from all notifiers in this context. */ synchronized void removeNamingListener(NamingListener l) { - Enumeration allnotifiers = notifiers.elements(); - NamingEventNotifier notifier; - if (debug) System.err.println("EventSupport removing listener"); // Go through list of notifiers, remove 'l' from each. // If 'l' is notifier's only listener, remove notifier too. - while (allnotifiers.hasMoreElements()) { - notifier = (NamingEventNotifier)allnotifiers.nextElement(); + for (NamingEventNotifier notifier : notifiers.values()) { if (notifier != null) { if (debug) System.err.println("EventSupport removing listener from notifier"); Additional information: the 'notifiers' field is a Hashtable. Later on in the loop, the code does notifiers.remove(notifier.info); This is the concurrent modification referred to in the description. Note: notifier.info is the Hashtable key corresponding to this notifier. One could change this to use the full form of the for-loop, exposing the Iterator, and then calling iterator.remove(). It might be safer, however, to restore this to use Enumeration. While concurrent modification to the underlying collection while using an Enumeration is completely unspecified, to my eye, it actually appears safe to remove elements from a Hashtable while using an Enumeration.
30-03-2017