> On Apr 12, 2017, at 2:48 PM, Stuart Douglas <sdouglas_at_redhat.com> wrote:
>
> On Thu, Apr 13, 2017 at 3:46 AM, Shing Wai Chan
> <shing.wai.chan_at_oracle.com> wrote:
>>
>>> On Apr 11, 2017, at 6:30 PM, Stuart Douglas <sdouglas_at_redhat.com> wrote:
>>>
>>> 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 agree that the `before’ seems to imply ‘happen-before’ here.
>> The issue 81 is asking for clarification of “before” as “happen-before”.
>>
>>>
>>> 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).
>>
>> I have took a quick look on some of the above implementations.
>> It seems that they have synchronization for creation of servlet.
>> But I am not sure about the invocation of servlet#service there.
>>
>
> You don't need to synchronize on the invocation of servlet#service
> (which would be a bad idea for obvious reasons). You just need some
> kind of memory barrier when you acquire the Servlet instance to invoke
> upon. Some implementations seem to use a straight synchronization,
> while others use synchronization combined with a volatile (double
> checked locking). Either way because this happens before the service
> method is invoked in the same thread there is a happens before
> relationship between the sync/volatile and invoking the service
> method, and the sync is coded to make sure there is a happens before
> between invoking the init method and the servlet being acquired.
Let me recap what we discuss.
We have
B - The init method completing
C - The service method begin invoked
we would like to have hb(B, C).
From 17.4.5 Happens-before Order, we need to have
(ii) B synchronizes-with a following action C
where synchronizes-with is defined in 17.4.4 which may include lock, volatile, etc.
I would like to clarify that some works (more than just synchronize the servlet creation; possibly with volatile) need to be done in the container in order to have (ii).
With this understanding, then we can go back to the question whether we want to put “happen-before” in the spec.
SD> 1) Do nothing and close the JIRA, with a note explaining that volatile
SD> is not necessary, as it is not possible to satisfy the relationship
SD> that init() happens before service() without also establishing a
SD> happens-before relationship that means that any field changes will be
SD> visible to the thread invoking the service() method.
SD> 2) Clarify at the platform level that Java EE containers will ensure
SD> there is a happens before relationship between the initialization of
SD> any component that is used by multiple threads and its initialization.
SD> The reason why I think that this should happen at the platform level
SD> is that if we just clarify at the Servlet level people might assume
SD> that because we explicitly specify the relationship and other
SD> specification don't that they need to use volatile in other components
SD> (e.g. JAX-RS endpoints).
SD> 3) Change the spec to say that there must be a happens-before
SD> relationship between init() and service() (possibly with a note that
SD> just because this relationship is explicitly specified it does not
SD> mean that you need to start worrying about happens-before in other
SD> specifications that don't explicitly state this).
Thanks.
Shing Wai Chan
>
> Stuart
>
>>>
>>> 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
>>>>>>
>>>>
>>