On Sat, Aug 19, 2017 at 1:53 PM, Martin Buchholz <marti...@google.com> wrote:
> Now I see that the code snippet in TimeUnit.timedWait is also in need of > fixing. Hmmmm .... > > public synchronized Object poll(long timeout, TimeUnit unit) > throws InterruptedException { > while (empty) { > unit.timedWait(this, timeout); > ... > } > } > > It's surprisingly hard to write a high quality wait loop, and we've been doing our users a disservice by punting on providing good models in the javadoc. You probably want to work entirely in nanoseconds, to avoid rounding errors, and because you'll end up calling nanoTime anyways. You don't want to call nanoTime unless you are sure to wait. OTOH you want to throw NPE on null TimeUnit even if there's no need to wait. You must beware of overflow when timeout is Long.MIN_VALUE. Below is my attempt to fix the sample loop in timedWait. Calling Object.wait directly is obviously even harder. How about just not providing a sample loop that calls Object.wait directly (effectively, gently deprecate timed Object wait in favor of the loop below) (I'm also pretending that the platform has better than millisecond granularity). public E poll(long timeout, TimeUnit unit) throws InterruptedException { long nanosTimeout = unit.toNanos(timeout); synchronized (lock) { for (long deadline = 0L; isEmpty(); ) { long remaining = (deadline == 0L) ? nanosTimeout : deadline - System.nanoTime(); if (remaining <= 0) return null; if (deadline == 0L) { // first wait deadline = System.nanoTime() + nanosTimeout; if (deadline == 0L) // unlikely corner case deadline = 1L; } NANOSECONDS.timedWait(lock, remaining); } return dequeueElement(); } }