Rob, > On 3 Sep 2018, at 22:48, Rob McKenna <rob.mcke...@oracle.com> wrote: > > Hi folks, > > I'd like to get a re-review of this change: > > https://bugs.openjdk.java.net/browse/JDK-8139965 > <https://bugs.openjdk.java.net/browse/JDK-8139965>
This issue is closed as `will not fix`. I presume you will re-open it before pushing. > http://cr.openjdk.java.net/~robm/8139965/webrev/ > <http://cr.openjdk.java.net/~robm/8139965/webrev/> 43 private boolean completed; Won’t `completed` need to be volatile now? ( since the removal of synchronized from hasSearchCompleted ) LdapRequest.close puts EOF into the queue, but that is a potentially blocking operation ( while holding the lock ). If the queue is at capacity, then it will block forever. This model will only work if `getReplyBer` is always guaranteed to be running concurrently. Is it? Please check the indentation of LdapRequest.java L103 ( in the new file ). It appears, in the webrev, that the trailing `}` is not lined up. The indentation doesn’t look right here either. 624 if (nparent) { 625 LdapRequest ldr = pendingRequests; 626 while (ldr != null) { 627 ldr.close(); 628 ldr = ldr.next; 629 } 630 } 631 } -Chris
