On 12/04/17 00:43, Stuart Douglas 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.
Having thought this through more carefully, I agree. I was forgetting
that even if Thread A performed the init, the results of that init - and
particularly the ServeltConfig - will always be visible to Thread B
because of what we have in place to ensure that:
a) init() is called before service()
b) init() is only called once.
> 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).
My preference would be for 1) or 2)
Mark