On 24/03/17 00:02, Stuart Douglas wrote:
> On Fri, Mar 24, 2017 at 10:44 AM, Edward Burns <edward.burns_at_oracle.com> wrote:
>>>>>>> On Wed, 22 Mar 2017 09:11:04 +1100, Stuart Douglas <sdouglas_at_redhat.com> said:
>>
>> SD> I think we should go with Option Container, it would very much
>> SD> surprise me if any containers did not already do this.
>>
>> SD> In practice the code that stops service() being called before init()
>> SD> is complete should be a sufficient memory barrier to make sure that
>> SD> the changes in init() are visible. Off the top of my head I can't
>> SD> think of a way to implement this such that the changes are not
>> SD> guaranteed to be visible without also opening up the possibility of
>> SD> init() and service() being active at the same time.
>>
>>>>>>> On Tue, 21 Mar 2017 22:11:14 +0000, Mark Thomas <markt_at_apache.org> said:
>>
>> MT> I prefer Option Container. This is the behaviour users tend to assume
>> MT> and it is going to be less work (overall) to fix this in a handful of
>> MT> containers than in every Servlet.
>>
>> Thanks Mark and Stuart for your prompt responses. I'm hesitant to
>> introduce new locking requirements at this point in our implementation
>> histories. For this reason, I'm going to take option Container off the
>> table and restate the other two options with alternate text. I recommend
>> we take option ServletAuthor. Note that I'm removing any explicit
>> reference to the ServletConfig object.'
>
> This is not locking behavior, this is a happens before requirement
> which is a different thing altogether (although locks are one way to
> implement this, they are far from the only way).
>
> Does any container actually not implement this? As I said above I
> don't think it is actually possible to implement this in a spec
> compliant way that does not satisfy the requirements of
> OptionContainer.
>
> As an example consider what happens when you have multiple requests
> come in for an uninitialized Servlet. This will require the init()
> method to be called by one (and only one thread), while the other
> threads need to wait for the init() method to finish. Whatever method
> is used to implement this wait (be it synchronized, or one of the
> constructs from java.util.concurrent) will be a sufficient memory
> barrier that there will be a happens before relationship between the
> init() method and any thread entering the Servlet.
>
> I might be wrong but at the moment I can't really think of a way to
> implement this that ensures the init() method has been completed
> without also establishing a happens before relationship between the
> init() method and any thread entering the service() method.
>
>
>>
>> Option WONTFIX:
>>
>> We simply punt on this issue and mark it as WONTFIX. We point out
>> that this is precisely the sort of thing the volatile keyword is
>> for. [1]
>
> If option container is not on the table then we should use this
> option, and not say anything, as in practice Servlet authors won't
> have to do anything.
>
>>
>> Option ServletAuthor:
>>
>> I propose we take the easy way out here and add the following text to
>> Servlet#init():
>>
>> Servlet Developers must make adequate provisions for the visibility
>> of shared variables (as defined in 17.4.1 of the Java Language
>> Specification) written to in the init() method and read from in the
>> service() method (or any of its convenience methods). For example,
>> the volatile keyword could be applied to any instance variables
>> written to during init() and read from during service().
>>
>> And the following sentence after the sentence in 2.3.3.1. New text in
>> **.
>>
>> 2.3.3.1 Multithreading Issues
>>
>> A servlet container may send concurrent requests through the service
>> method of the servlet. To handle the requests, the Servlet Developer
>> must make adequate provisions for concurrent processing with multiple
>> threads in the service method. **Servlet Developers must make
>> adequate provisions for the visibility of shared variables (as defined
>> in 17.4.1 of the Java Language Specification) written to in the init()
>> method and read from in the service() method (or any of its
>> convenience methods). For example, the volatile keyword could be
>> applied to any instance variables written to during init() and read
>> from during service().**
>
> This has a big -1 from me. Instead of adding a new requirement on the
> container you have now added a new requirement on Servlet authors and
> now potentially millions of existing Servlet applications will no
> longer be spec compliant according to the letter of the spec. You said
> above "I'm hesitant to introduce new locking requirements at this
> point in our implementation histories." but this is exactly what you
> are doing, except that this affects end users instead of just
> container implementations.
>
> Concurrency is hard, and this is exactly the sort of concurrency issue
> that Java EE is supposed to solve, we should not be punting it onto
> application developers when it is trivial for us to make sure the
> containers do the correct thing.
+1
I couldn't agree more with everything Stuart has written.
Mark