JEP 264: Platform Logging API and Service
Roger Riggs
Roger.Riggs at Oracle.com
Tue Oct 6 13:53:46 UTC 2015
Hi,
This case does not require a restriction. This is not really a case of
a singleton object.
I think the statement is a statement of fact.
The behavior should be attributed to the LoggerFinder.getLoggerFinder method
and not to the constructor of LoggerFinder.
Roger
On 10/6/2015 9:32 AM, Peter Levart wrote:
>
>
> 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
>>>>
>>>
>>
>
More information about the core-libs-dev
mailing list