jsr369-experts@servlet-spec.java.net

[jsr369-experts] Re: [81-ClarifyThreadRequirementsOnInit] DISCUSSION

From: Stuart Douglas <sdouglas_at_redhat.com>
Date: Wed, 12 Apr 2017 11:30:49 +1000

It is up to the Servlet container to ensure there is a happens-before
relationship between init() finishing and service() being called. Even
though the spec actually just says 'before' and does not explicitly
state that it is a 'happens-before' relationship as per JSR-133 I
think that this is really the only way you can define 'before' with
regards to multiple threads on the JVM.

I examined the implementations of Undertow, Jetty, Tomcat and
Glassfish and it appears that they all use the synchronized keyword to
achieve this, in some cases combined with double checked locking to
minimize the performance impact (so the lock will only be taken while
init is running, otherwise the only cost is a volatile read).

There are other ways to achieve this, namely by using classes from the
java.util.concurrent package (which provide their own happens before
guarantees, usually specified in the javadoc of the class).

Stuart


On Wed, Apr 12, 2017 at 10:35 AM, Shing Wai Chan
<shing.wai.chan_at_oracle.com> wrote:
>
>> On Apr 11, 2017, at 4:43 PM, Stuart Douglas <sdouglas_at_redhat.com> wrote:
>>
>> On Wed, Apr 12, 2017 at 8:34 AM, Mark Thomas <markt_at_apache.org> wrote:
>>> On 11/04/17 16:42, Edward Burns wrote:
>>>>>>>>> On Fri, 24 Mar 2017 09:51:38 +0000, Mark Thomas <markt_at_apache.org> said:
>>>> SD> Concurrency is hard, and this is exactly the sort of concurrency issue
>>>> SD> that Java EE is supposed to solve, we should not be punting it onto
>>>> SD> application developers when it is trivial for us to make sure the
>>>> SD> containers do the correct thing.
>>>>
>>>> MT> +1
>>>>
>>>> MT> I couldn't agree more with everything Stuart has written.
>>>>
>>>>>>>>> On Mon, 10 Apr 2017 12:09:36 -0700, Edward Burns <edward.burns_at_oracle.com> said:
>>>>
>>>> EB> Shing-wai looked into the GlassFish implementation and suggested we
>>>> EB> simply leave the issue in the JIRA and look to resolve it in a future
>>>> EB> release. I will take this option.
>>>>
>>>> In case this was not clear, I also agree with this resolution of leaving
>>>> the issue in JIRA for 4.0.
>>>
>>> I'm not sure why this is being pushed out to the next spec version.
>>> Isn't this as simple as making the ServletConfig field in GenericServlet
>>> volatile?
>>
>> No, making fields volatile will not help at all.
>>
>> At the moment the spec says: "After the servlet object is
>> instantiated, the container must initialize the servlet before it can
>> handle requests from clients." It is not actually possible to meet
>> this condition without also establishing a happens-before relationship
>> that guarantees any writes from the init() method are visible to the
>> thread calling service() (see below for the logic behind this).
>>
>> This means adding volatile does nothing, as either the container
>> implements the specification and it is unnecessary, or it does not and
>> init() may not have finished, so volatile will not help as the write
>> may not have actually happened yet.
>>
>> IMHO we should do one of the following:
>>
>> 1) Do nothing and close the JIRA, with a note explaining that volatile
>> is not necessary, as it is not possible to satisfy the relationship
>> that init() happens before service() without also establishing a
>> happens-before relationship that means that any field changes will be
>> visible to the thread invoking the service() method.
>>
>> 2) Clarify at the platform level that Java EE containers will ensure
>> there is a happens before relationship between the initialization of
>> any component that is used by multiple threads and its initialization.
>> The reason why I think that this should happen at the platform level
>> is that if we just clarify at the Servlet level people might assume
>> that because we explicitly specify the relationship and other
>> specification don't that they need to use volatile in other components
>> (e.g. JAX-RS endpoints).
>>
>> 3) Change the spec to say that there must be a happens-before
>> relationship between init() and service() (possibly with a note that
>> just because this relationship is explicitly specified it does not
>> mean that you need to start worrying about happens-before in other
>> specifications that don't explicitly state this).
>>
>>
>> The formal Java Memory Model reasoning (see
>> http://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4.5
>> )
>>
>> According to the model the two bits we are interested in is:
>>
>> (1) If x and y are actions of the same thread and x comes before y in
>> program order, then hb(x, y).
>> (2) If hb(x, y) and hb(y, z), then hb(x, z).
>>
>> And from the Servlet spec we have:
>>
>> (3) After the servlet object is instantiated, the container must
>> initialize the servlet before it can handle requests from clients.
>>
>> If we define the following actions:
>> A - The field write in the init method
>> B - The init method completing
>> C - The service method being invoked
>>
>> We know that we have the following:
>>
>> hb(A, B) - the field write has to happen before the init method
>> completes, as per (1)
>> hb(B, C) - init is finished before calling service(), as per (3)
>
> My question is on hb(B, C).
> From 17.4.5, we can have hb(B, C) if
> (i) B and C are on the same thread and B comes before C
> (ii) B synchronizes-with a following action C
> (iii) there is a D such that hb(B, D) and hb(D, C)
> (iv) B before Thread.start().
>
> Since B and C may be on different threads, we only have (ii) and (iii).
> ( (iv) may require using new thread for those requests. This may be not easy to do.)
> (ii) may request a synchronization/lock. Is it a concern here?
> Or can we find D and use (iii)?
>
> Thanks.
> Shing Wai Chan
>
>>
>> We can then infer:
>> hb(A, C) - the field write has to happen before the service method is
>> called, as per (2)
>>
>> Stuart
>>
>>>
>>> Mark
>>>
>