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.