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."