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