Are you talking about making this change in signalCompletionIfShutdown()? If so I am OK with what you are proposing.
Ralph > On Sep 12, 2015, at 11:19 AM, Remko Popma <[email protected]> wrote: > > I want to run this by you: > > LoggerConfig currently always takes a lock after logging a log event. > Within this lock it checks if the shutdown flag has been set. > Can we do it the other way around? First check if the shutdown flag has been > set and then take the lock (to notify the other thread)? > > I think it is safe to do this: > > public void log(final LogEvent event) { > counter.incrementAndGet(); > try { > if (!isFiltered(event)) { > processLogEvent(event); > } > } finally { > // FIRST update counter, THEN check shutdown flag; > // these are atomic and will not get re-ordered. > // So we don't need the lock to check the shutdown flag. > if (counter.decrementAndGet() == 0 && shutdown.get()) { > // ONLY take the lock if the shutdown flag was set > shutdownLock.lock(); > try { > noLogEvents.signalAll(); // lock required to call signal() > } finally { > shutdownLock.unlock(); > } > } > } > } > > // keep this method unchanged > private void waitForCompletion() { > shutdownLock.lock(); > try { > // NOTE: FIRST the shutdown flag is set > if (shutdown.compareAndSet(false, true)) { > int retries = 0; > // NOTE: THEN the counter is read > while (counter.get() > 0) { > try { > noLogEvents.await(retries + 1, TimeUnit.SECONDS); > } catch (final InterruptedException ie) { > if (++retries > MAX_RETRIES) { > break; > } > } > } > } > } finally { > shutdownLock.unlock(); > } > } > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
