On 10/05/2015 09:59 PM, Daniel Fuchs wrote:
Thanks Roger!
I have updated the specdiff online.
http://cr.openjdk.java.net/~dfuchs/8046565/proto.01/specdiff-api/overview-summary.html
The only comment I haven't taken into account yet is this:
> - the LoggerFinder constructor says "Only one instance will be
created".
> That applies to the normal static logger initialization
> (getLoggerFinder).
> But there might be other cases where the application or service
> might create a LoggerFinder
> for its own purposes, and the phrase is not accurate in that case.
I was wondering whether I should try enforcing this actually, by
throwing a ServiceConfigurationError or whatever if the
LoggerFinder service is already loaded when the constructor
is invoked.
We had trouble in the past with LogManager - because the spec
said there should be only one instance, but the implementation
did not enforce it.
It may be a bit different with LoggerFinder - as this is an
abstract class devoid of instance state (the only method with a
body is getLocalizedLogger and that's stateless) - so there may
not be as much harm as with LogManager.
There is probably no good way of implementing such enforcement
though - so it would be a best effort :-(
Hi Daniel,
Scala has singleton objects. Java hasn't. Your statement intrigued me
to think whether it would be possible to enforce a
one-instance-per-concrete-class rule in Java using just API. Here's a
prototype that I think does that. Can you think of a corner case that
fools it?
/**
* An abstract base class for subclasses that can only have one instance
* per concrete subclass. Subclasses must define a public no-args
constructor
* which must never be called directly from code (it throws
* UnsupportedOperationException), but via the factory method:
* {@link #getInstance(Class)}.
*/
public abstract class ClassObject {
/**
* Lazily constructs and returns a singleton instance per given
concrete
* {@code ClassObject} subclass or throws exception.
* Subclasses must define a public no-argument constructor. Multiple
* invocations of this method with same {@code clazz} parameter
either return
* the same instance or throw the same exception. The result of
this method
* is therefore stable for given parameter.
*
* @param clazz the Class representing concrete {@code
ClassObject} subclass
* @param <T> the type of the {@code ClassObject} subclass
* @return a singleton instance for given {@code clazz}
* @throws InstantiationException see {@link Constructor#newInstance}
* @throws IllegalAccessException see {@link Constructor#newInstance}
* @throws InvocationTargetException see {@link
Constructor#newInstance}
*/
public static <T extends ClassObject> T getInstance(Class<T> clazz)
throws InstantiationException, IllegalAccessException,
InvocationTargetException {
return clazz.cast(factoryCV.get(clazz).get());
}
/**
* ClassObject constructor allows construction of a single
instance of
* {@code ClassObject} subclass per concrete subclass.
*/
public ClassObject() {
Factory factory = factoryCV.get(getClass());
synchronized (factory) {
if (!factory.inConstruction) {
throw new UnsupportedOperationException(
"Direct construction of ClassObject instances is
not supported." +
" Please use ClassObject.getInstance(Class)
instead.");
}
if (factory.constructed != null) {
throw new IllegalStateException(
"Attempt to instantiate a second " +
getClass().getName() + " instance.");
}
factory.constructed = this;
}
}
/**
* A ClassValue cache of Factory instances per class
*/
private static final ClassValue<Factory> factoryCV = new
ClassValue<Factory>() {
@Override
protected Factory computeValue(Class<?> clazz) {
return new Factory(clazz.asSubclass(ClassObject.class));
}
};
/**
* A Factory responsible for constructing and caching a singleton
instance
* of specified class.
*/
private static final class Factory {
// the class of instance to construct
private final Class<? extends ClassObject> clazz;
// published instance or Throwable
private volatile Object instance;
// temporarily set to true while constructing and checked in
ClassObject constructor
boolean inConstruction;
// the just being constructed instance or Throwable, set in
ClassObject constructor
Object constructed;
Factory(Class<? extends ClassObject> clazz) {
this.clazz = clazz;
}
ClassObject get() throws InstantiationException,
IllegalAccessException, InvocationTargetException {
Object obj = instance;
if (obj == null) {
synchronized (this) {
obj = instance;
if (obj == null) {
if (constructed == null) {
try {
Constructor<? extends ClassObject>
constructor =
clazz.getDeclaredConstructor();
inConstruction = true;
constructor.newInstance();
} catch (Throwable t) {
// override with construction
exception if thrown
constructed = t;
} finally {
inConstruction = false;
}
}
instance = obj = constructed;
assert obj != null;
}
}
}
if (obj instanceof ClassObject) {
return (ClassObject) obj;
}
Factory.<InstantiationException>sneakyThrow((Throwable) obj);
// not reached
return null;
}
@SuppressWarnings("unchecked")
private static <T extends Throwable> void
sneakyThrow(Throwable t) throws T {
throw (T) t;
}
}
}
I don't think it is very important to enforce such rule for services
provided via ServiceLoader since similar effect can be achieved by
treating the published abstract type only as a factory for a real
service which can then be initialized and cached as a singleton object
in the implementation itself. But that can not be enforced on the
system level. If the abstract type has state and you would want to
initialize it only once, ClassObject might be an answer. If
ClassObject abstract class was part of JDK, ServiceLoader would have
to special-case it's subclasses and invoke
ClassObject.getInstance(Class) instead of the no-arg constructor to
obtain the instance.
Regards, Peter
best regards,
-- daniel
On 05/10/15 16:06, Roger Riggs wrote:
Hi Daniel,
This looks good, a few comments...
j.l.System:
- the behaviors described by the @implNote in getLogger(name) and
@implSpec in getLogger(name, resourceBundle) seem like they should
be consistent
for the two methods.
System.Logger:
- log(level, throwable, Supplier) - to be more consistent, the
Suppler<String> argument
should be before the Throwable argument.
Then in all cases the msg/string is before the Throwable.
System.Logger.Level
- TRACE might be used for more than just method entry and exit,
perhaps the description can be more
general: "usually used for diagnostic information."
LoggerFinder:
- should the CONTROL_PERMISSION field be renamed to
"LOGGERFINDER_PERMISSION"?
- the LoggerFinder constructor says "Only one instance will be
created".
That applies to the normal static logger initialization
(getLoggerFinder).
But there might be other cases where the application or service
might create a LoggerFinder
for its own purposes, and the phrase is not accurate in that case.
Editorial:
- The @since tags can be set to "9".
- "JVM" -> "Java runtime"; JVM would specifically refer to the
virtual machine implementation. (j.u.System.LoggerFinder)
- "used by the JDK" can be omitted. (j.u.System.LoggerFinder)
When used in the context of the JDK documentation itself it seems
out of place and is unnecessary.
- the word 'actually' can/should be omitted from descriptions.
(j.l.System)
Thanks, Roger
On 10/5/2015 6:54 AM, Daniel Fuchs wrote:
New JEP Candidate: http://openjdk.java.net/jeps/264
Hi I have uploaded an initial prototype specdiff:
http://cr.openjdk.java.net/~dfuchs/8046565/proto.01/specdiff-api/overview-summary.html
LoggerFinder methods that take a Class<?> caller argument
will take a java.lang.reflect.Module callerModule instead when
that is available in jdk9/dev.
comments welcome,
best regards,
-- daniel
