On 09/05/2013 10:08 AM, Daniel Fuchs wrote:
Or, without using yet another class, double-checked locking, like:
private volatile boolean initializedCalled;
final void ensureLogManagerInitialized() {
if (initializedCalled) {
return;
}
synchronized (this) {
final LogManager owner = this;
// re-check under lock
if (initializedCalled || owner != manager) {
// we don't want to do this twice, and we don't want to do
// this on private manager instances.
return;
}
try {
AccessController.doPrivileged(....);
} finally {
initializedCalled = true;
}
}
}
Ah! Thanks for that comment Peter - that's exactly what I had in mind.
... the code in PrivilegedAction that does the initialization, that
is, the following statements:
owner.readPrimordialConfiguration();
* owner.rootLogger = owner.new RootLogger();*
owner.addLogger(owner.rootLogger);
final Logger global = Logger.global;
owner.addLogger(global);
...almost all of them (except assignment to rootLogger) by themselves
ensure that the state mutations they make are published to other
threads, so if also *rootLogger* field was made volatile, such
double-checked locking would presumably not be broken.
Hmmm... By that I assume you mean the operations that access
LogManager.rootLogger
from LoggerContext. AFAIR all these operations are synchronized on
LoggerContext,
The fact that they are synchronized on LoggerContext does not prevent
publication of rootLogger to them without write-read edge, since writing
to rootLogger field is performed without synchronization on the same
LoggerContext (it is performed with synchronization on the LogManager
instance)...
and will trigger a call to ensureLogManagerInitialized().
ensureLogManagerInitialized() is called for every add/get logger
operation.
If ensureLogManagerInitialized() is executing other threads will be
blocked.
When it finishes - threads that were waiting on the monitor will find the
initializationCalled flag set to true and will simply return.
Ok.
Threads that call ensureLogManagerInitialized() after that will find
the flag
is true before entering the synchronized block and will return directly.
...and such threads can see rootLogger field being either still null or
not null but the Logger instance that is obtained via such data race can
be half-initialized.
I can make rootLogger volatile. I'm not 100% sure it is required - but
I think
it is more correct, and as such it will make the code more maintainable.
Good point. It's probably something I would have overlooked.
I think volatile rootLogger field is required for correct publication of
RootLogger instances if double-checked locking is used...
Regards, Peter
