Hi Chris,
Thanks for taking care of this followup fix.
I see at least three other places where `notifiers` is accessed
without a null check. It might be good to fix these three other
places too just in case.
(two addNamingListener methods and 1 removeNamingListener)
All of them are already synchronized so a simple null check should
be enough.
Also you could had the @bug 8241130
to RemoveNamingListenerTest.java, since the test can be used
to verify the fix (if run often enough).
best regards,
-- daniel
On 18/03/2020 08:16, Chris Yin wrote:
Hello
Please review following changes to fix corner issue
in com.sun.jndi.ldap.EventSupport, thanks
Bug: https://bugs.openjdk.java.net/browse/JDK-8241130
Webrev: http://cr.openjdk.java.net/~xyin/8241130/webrev.00/
This is a follow up item which related to 8202117. When testing the
second revision fix
for 8202117 (http://mail.openjdk.java.net/pipermail/core-libs-dev/2020-March/065270.html),
intermittent failure with java.lang.NullPointerException been
observed(Thanks Daniel for additional testing), that exposed another
issue in com.sun.jndi.ldap.EventSupport.
“com.sun.jndi.ldap.NamingEventNotifier" is a background thread which
been created when addNamingListener with ldap context, per code logic,
it will call EventSupport.removeDeadNotifier when encounter
NamingException (maybe wrapped a SocketException) in some case, if user
closed source ldap context first (EventSupport.cleanup been triggered),
the call to EventSupport.removeDeadNotifier will
throw NullPointerException since var “notifiers” already been set to
null in previous EventSupport.cleanup. Maybe in real world, it’s hard to
get all criteria been satisfied to reproduce, but it’s still a potential
multithreading issue and caught by test with repeat run.
Combined this change and 8202117 second revision change, run test
com/sun/jndi/ldap/RemoveNamingListenerTest.java on 4 platforms for total
2000 times, no failure been observed, also tier1, tier2, tier3 tests passed.
Thanks,
Chris
