user manual code example race condition

[Alan Ruttenberg]

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 at panix.com> wrote:

>
>
> > On Mar 11, 2019, at 16:18, Pascal Bourguignon <pjb at informatimago.com>
> wrote:
> >
> >
> >
> >> On 11 Mar 2019, at 10:59, Mark Evenson <evenson at panix.com> wrote:
> >>
> >>
> >>
> >>> On Mar 11, 2019, at 06:38, dingd <ntysdd at 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."
>
>
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mailman.common-lisp.net/pipermail/armedbear-devel/attachments/20190311/ae3abc57/attachment.html>