I haven't looked at this code before, but in looking now, I am confused.
Interpreter is intended to be a singleton, right? (that's what the comment says, as well as what is suggested by the use of the static interpreter variable.).
In that case why would you want to createInstance to ever return null? 
Why isn't there just a single synchronized getSingleton() (call it getInterpreter if you want) method, which either returns returns the existing interpreter, or creates and initializes a new one and returns it.

Alan

On Mon, Mar 11, 2019 at 1:06 PM Mark Evenson <evenson@panix.com> wrote:


> On Mar 11, 2019, at 16:18, Pascal Bourguignon <pjb@informatimago.com> wrote:
>
>
>
>> On 11 Mar 2019, at 10:59, Mark Evenson <evenson@panix.com> wrote:
>>
>>
>>
>>> On Mar 11, 2019, at 06:38, dingd <ntysdd@qq.com> wrote:
>>>
>>> this is recommended in the manual, it contains a race condition.
>>>
>>> Interpreter interpreter = Interpreter.getInstance ();
>>> if ( interpreter == null ) {
>>>     interpreter = Interpreter.createInstance ();
>>> }
>>>
>>> getInstance and createInstance should be swapped.
>>
>> Since both Interpreter.getInstance() and Interpreter.createInstance() are
>> methods synchronized on the same object monitor, I don’t see that there is a
>> race condition here.  Could you explain a little more about your reasoning
>> and/or experience with the race condition?
>
>
> Two threads could call getInstance and have interpreter=null.
> Then both will call createInstance, thus two instances will be created.
> If interpreter is a local binding as it appears in the code above, then it is what’s intended and all is good.
> But if interpreter is a global binding and you expected interpreter to have a single instance, then it’s wrong.

I am not sure what you mean by global/local bindings here, as these concepts
don’t exist in Java as far as I know,  but I think dingd is correct here:  I
don’t think two instances of the interpreter will be created.

As the Interpreter.createInstance() is synchronized on the Interpreter class
(the “object monitor”), one of the calls to createInstance() will succeed in
creating a value in the static Interpreter.interpreter reference, while the
other will return null.


 // Interface.
    public static synchronized Interpreter createInstance()
    {
        if (interpreter != null)
            return null;
        interpreter = new Interpreter();
        _NOINFORM_.setSymbolValue(T);
        initializeLisp();
        return interpreter;
    }

I’m still scratching my head over why the current implementation is so
convoluted.   Part of the problem is that we have three methods of creating an
interpreter namely, createInstance(), createDefaultInstance() and
createJLispInstance() with two different possible private constructors.  If
anyone has a good handle on the right architecture here, please pipe up. 

The simplest path forward would be to change the documentation, as dingd
originally suggested.  But that path isn’t very satisfying.


--
"A screaming comes across the sky.  It has happened before but there is nothing
to compare to it now."