You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomcat.apache.org by Péter Gergely Horváth <pe...@gmail.com> on 2016/11/25 13:32:00 UTC
Thread-safety of javax.servlet.Servlet#getServletConfig()
Hi All,
I am wondering why calling javax.servlet.Servlet#getServletConfig() is
thread safe: if you check the implementation in
javax.servlet.GenericServlet from servlet API 3.0.1, you see the following:
package javax.servlet;
// lines omitted
public abstract class GenericServlet
implements Servlet, ServletConfig, java.io.Serializable
{
// lines omitted
private transient ServletConfig config;
// lines omitted
public void init(ServletConfig config) throws ServletException {
this.config = config;
this.init();
}
// lines omitted
public ServletConfig getServletConfig() {
return config;
}
// lines omitted
}
The field config is written in init(ServletConfig) without any
synchronization, locking or config being volatile, while getServletConfig()
could be called anytime from any later worker thread of the servlet
container. I took a quick look into Tomcat/Catalina code base, however I
see no indication of the servlet container performing any magic, which
would guarantee thread safe access to field config through
getServletConfig() from e.g.
javax.servlet.GenericServlet#service(ServletRequest, ServletResponse) and
the corresponding methods in javax.servlet.http.HttpServlet.
Am I missing something here? What will guarantee that if Thread A is used
to init(ServletConfig) then Thread B calling getServletConfig() will see
the correct state of the field config (Java "happens-before"), and not e.g.
null?
I am asking this because I have seen some legacy code, where a servlet
stored something into a field inside the init() method, which was then used
from within a javax.servlet.http.HttpServlet#doGet or
javax.servlet.http.HttpServlet#doPost method, without synchronization of
any kind like this:
public class FoobarServlet extends HttpServlet {
private FoobarService foobarService;
@Override
public void init() throws ServletException {
this.foobarService = // ... acquire foobarService
}
protected void doGet(HttpServletRequest request, HttpServletResponse
response) throws ServletException, IOException {
List<Foobar> foobars = this.foobarService.getFoobars(); // read the
field WITHOUT synchronization
// ...
}
// ...
Is this approach (having no synchronization, locking or the field being
volatile) correct? I assumed it is not, seeing something like that in the
servlet API is quite confusing.
What do you think?
Thanks,
Peter
Re: Thread-safety of javax.servlet.Servlet#getServletConfig()
Posted by "Terence M. Bandoian" <te...@tmbsw.com>.
On 12/5/2016 7:40 AM, P�ter Gergely Horv�th wrote:
> Hi Chris,
>
> Thanks your four input: this question is somewhere in-between... :)
>
> We have *definitely* seen cases, where a piece of code like the one below
> sometimes (a couple of times from tens of thousands of successfully
> serviced requests) found the stored field to be null - with a
> NullPointerException being thrown on the first access of the fooBarService
> field.
>
> @WebServlet("/open")
> public class FooBarServlet extends HttpServlet {
>
> private FooBarService fooBarService;
> @Override
> public void init() throws ServletException {
> try {
> ApplicationContext applicationContext =
> (ApplicationContext)
> getServletContext().getAttribute("springContext");
> fooBarService =
> applicationContext.getBean("fooBarService",
> FooBarService.class);
>
> } catch (Exception ex) {
> // wrap any exceptions to ServletException so as to comply with
> Servlet contract
> throw new ServletException("Initialization failed with
> exception", ex);
Does this exception ever occur?
-Terence Bandoian
> }
> }
>
> protected void doGet(HttpServletRequest request, HttpServletResponse
> response) throws ServletException, IOException {
> processRequest(request, response);
> }
>
> protected void doPost(HttpServletRequest request, HttpServletResponse
> response) throws ServletException, IOException {
> processRequest(request, response);
> }
>
> private void processRequest(HttpServletRequest request,
> HttpServletResponse response) throws IOException, ServletException {
>
> List<FooBar> = fooBarService.getFooBars(); /// we have seen NPEs
> thrown from here
> /// ...
> }
>
>
> For the first glance, it seemed to be obvious that it must have been a
> threading issue, which could easily be solved by adding volatile to the
> stored fooBarService field.
>
> However, I was more than confused seeing
> that javax.servlet.GenericServlet#config uses the same approach by simply
> storing the ServletConfig into a field and reading it later on without any
> locking etc.
>
> I quickly scanned through the Servlet specs and Catalina codes, but cannot
> really locate any explicit reference to thread-safety of
> javax.servlet.GenericServlet#getServletConfig, that is where the question
> originates.
>
> I would by much obliged if you had any inputs, ideas regarding this, or on
> the approach one could take to confirm it on his/her own.
>
> Thanks,
> Peter
>
>
> On Mon, Nov 28, 2016 at 6:45 PM, Christopher Schultz <
> chris@christopherschultz.net> wrote:
>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA256
>>
>> P�ter,
>>
>> On 11/28/16 11:01 AM, P�ter Gergely Horv�th wrote:
>>> Thank you very much for your feedback, but I think there is a
>>> slight misunderstanding here: I know the order in which a servlet
>>> is initialized and put into service, and this question is not
>>> related to that. It's related to the _visibility_ of the field.
>>>
>>> The question is about the thread safety of the field access: if
>>> you carefully check the details of
>>> javax.servlet.GenericServlet#config and compare the implementation
>>> to the sample "NoVisibility" from the book Java Concurrency in
>>> Practice, then it is seems to follow the same pattern, which is
>>> "not thread-safe because the value field is accessed from both get
>>> and set without synchronization. Among other hazards, it is
>>> susceptible to stale values: if one thread calls set, other threads
>>> calling get may or may not see that update." [1].
>>>
>>> Personally, I would like to understand why this implementation is
>>> not (if not) susceptible to the stale values phenomenon [2]; there
>>> might be someone, who can point me to the right direction.
>> I think you are correct that there is a theoretical multi-threaded
>> race-condition, here. The container may initialize the servlet in one
>> thread and service e.g. the first request in another thread, and the
>> ServletContext reference might not be written to main memory and then
>> read-back by the request-processing thread.
>>
>> Potential fixes for this would be either to define the ServletContext
>> member to be volatile or to use synchronized methods.'
>>
>> Adding synchronization to the init() method would not be a problem at
>> all: there is very little monitor contention at that stage and the
>> container would only expect to call that method a single time. Adding
>> synchronization to the getServletContext method, though, might
>> represent a reduction in performance, since getServletContext gets
>> called many times during the lifetime of a Servlet, and from many thread
>> s.
>>
>> Likewise, marking the ServletContext member as "volatile" would
>> necessarily require a slow main-memory read every time
>> getServletContext is called.
>>
>> I suspect simple analysis above is the reason for no multithreaded
>> protection being placed on the ServletContext member in question.
>>
>> P�ter, is this an academic discussion, or have you in fact seen a case
>> where a servlet has been initialized and yet the first thread to
>> process a request receives a null value when calling getServletContext?
>>
>> - -chris
>>
>>> On Mon, Nov 28, 2016 at 6:08 AM, andreas <an...@junius.info>
>>> wrote:
>>>
>>>> ---- On Fri, 25 Nov 2016 23:02:00 +0930 P�ter Gergely Horv�th
>>>> wrote ----
>>>>> Hi All,
>>>>>
>>>>> I am wondering why calling
>>>>> javax.servlet.Servlet#getServletConfig() is thread safe: if you
>>>>> check the implementation in javax.servlet.GenericServlet from
>>>>> servlet API 3.0.1, you see the
>>>> following:
>>>>> package javax.servlet;
>>>>>
>>>>> // lines omitted
>>>>>
>>>>> public abstract class GenericServlet implements Servlet,
>>>>> ServletConfig, java.io.Serializable { // lines omitted
>>>>>
>>>>> private transient ServletConfig config;
>>>>>
>>>>> // lines omitted
>>>>>
>>>>> public void init(ServletConfig config) throws ServletException
>>>>> { this.config = config; this.init(); }
>>>>>
>>>>> // lines omitted
>>>>>
>>>>> public ServletConfig getServletConfig() { return config; } //
>>>>> lines omitted }
>>>>>
>>>>>
>>>>> The field config is written in init(ServletConfig) without any
>>>>> synchronization, locking or config being volatile, while
>>>> getServletConfig()
>>>>> could be called anytime from any later worker thread of the
>>>>> servlet container. I took a quick look into Tomcat/Catalina
>>>>> code base, however I see no indication of the servlet container
>>>>> performing any magic, which would guarantee thread safe access
>>>>> to field config through getServletConfig() from e.g.
>>>>> javax.servlet.GenericServlet#service(ServletRequest,
>>>>> ServletResponse) and the corresponding methods in
>>>>> javax.servlet.http.HttpServlet.
>>>>>
>>>>> Am I missing something here? What will guarantee that if Thread
>>>>> A is used to init(ServletConfig) then Thread B calling
>>>>> getServletConfig() will see the correct state of the field
>>>>> config (Java "happens-before"), and not
>>>> e.g.
>>>>> null?
>>>>>
>>>>> I am asking this because I have seen some legacy code, where a
>>>>> servlet stored something into a field inside the init() method,
>>>>> which was then
>>>> used
>>>>> from within a javax.servlet.http.HttpServlet#doGet or
>>>>> javax.servlet.http.HttpServlet#doPost method, without
>>>>> synchronization of any kind like this:
>>>>>
>>>>> public class FoobarServlet extends HttpServlet {
>>>>>
>>>>> private FoobarService foobarService;
>>>>>
>>>>> @Override public void init() throws ServletException {
>>>>> this.foobarService = // ... acquire foobarService } protected
>>>>> void doGet(HttpServletRequest request, HttpServletResponse
>>>>> response) throws ServletException, IOException { List<Foobar>
>>>>> foobars = this.foobarService.getFoobars(); // read the field
>>>>> WITHOUT synchronization // ... } // ... Is this approach
>>>>> (having no synchronization, locking or the field being
>>>>> volatile) correct? I assumed it is not, seeing something like
>>>>> that in the servlet API is quite confusing.
>>>>>
>>>>>
>>>>> What do you think?
>>>>>
>>>>> Thanks, Peter
>>>>
>>>> A Servlet will process requests only if it is fully initialized,
>>>> i.e. init has been executed. The init method gets called only
>>>> once and the servlet config won't change afterwards. I don't
>>>> think there is need for synchronization. The same is probably
>>>> valid for your own objects. Problems arise when individual
>>>> requests change the state of these objects.
>>>>
>>>> Andy
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>>
>>>>
>> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
>>>> For additional commands, e-mail: users-help@tomcat.apache.org
>>>>
>>>>
>> -----BEGIN PGP SIGNATURE-----
>> Comment: GPGTools - http://gpgtools.org
>> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>>
>> iQIcBAEBCAAGBQJYPG0dAAoJEBzwKT+lPKRYkLUP/3JrIPCjIsmw4o71nmZUeN4r
>> 0rt1ccflgjFUmpff5Pt9kHWJR60tga+sFT0jQ7LEvggbua1SoXV4y4MDINgEQ/Uu
>> ek0aiDRR0n8xXxUWcx7P8Jjmn5yyOGUrFVw029dD55XT6ArYz7gaAUnowRKB6DR+
>> RT84yO14TlRZx8DDLoxegmUpmXDpoHZyPKGGYWDhNepiU00X1YmFg/G3CiXJEjzd
>> B3GBewhE8nh9GZsqfg7U1V9EBQI+8jeI/GxRTSnsUwWDKvH7Ja6ffiLtAS+3mnWD
>> Ad2/wTbY7RT9ybT+thgvXkKJEKx+rVZrazb7QSgqq8Otv/aeMVLh16z2sxt6uuMy
>> fffKRwNUL12cKr8KdNvUnb4PVStVF8fnRxAW1M90axzFvxeoLOJJZXtS6TBy+MjE
>> eeMbjFV2wJw2y+66YWrvyCglyQU4c2ab5H0JuBINJ5oD1DSdTks0dW2rZvIm6pYp
>> kCQTLSbZFjePksoNw6gxDo5XQW1SczWbTG2lYwEjfm3eeyHRnaRgiV4LPBPsx5Zy
>> 7QFZJVGSFug2pgKTKErugoQs8z4gFCrryvebbGYgtvlsdJRk5Ldnl6uG0KabAEVU
>> PedppbSTPPkE8Ym+/YTTpJ4Bo5NMjkoYySWnWpBLof5sJ1hkjkURc4cBiFw4vqZO
>> K7WcPH4LDk5J5HzFiTIS
>> =DLPe
>> -----END PGP SIGNATURE-----
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: users-help@tomcat.apache.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Thread-safety of javax.servlet.Servlet#getServletConfig()
Posted by Mark Thomas <ma...@apache.org>.
On 05/12/2016 13:40, P�ter Gergely Horv�th wrote:
> Hi Chris,
>
> Thanks your four input: this question is somewhere in-between... :)
>
> We have *definitely* seen cases, where a piece of code like the one below
> sometimes (a couple of times from tens of thousands of successfully
> serviced requests) found the stored field to be null - with a
> NullPointerException being thrown on the first access of the fooBarService
> field.
>
> @WebServlet("/open")
> public class FooBarServlet extends HttpServlet {
>
> private FooBarService fooBarService;
> @Override
> public void init() throws ServletException {
> try {
> ApplicationContext applicationContext =
> (ApplicationContext)
> getServletContext().getAttribute("springContext");
> fooBarService =
> applicationContext.getBean("fooBarService",
> FooBarService.class);
>
> } catch (Exception ex) {
> // wrap any exceptions to ServletException so as to comply with
> Servlet contract
> throw new ServletException("Initialization failed with
> exception", ex);
> }
> }
>
> protected void doGet(HttpServletRequest request, HttpServletResponse
> response) throws ServletException, IOException {
> processRequest(request, response);
> }
>
> protected void doPost(HttpServletRequest request, HttpServletResponse
> response) throws ServletException, IOException {
> processRequest(request, response);
> }
>
> private void processRequest(HttpServletRequest request,
> HttpServletResponse response) throws IOException, ServletException {
>
> List<FooBar> = fooBarService.getFooBars(); /// we have seen NPEs
> thrown from here
> /// ...
> }
>
>
> For the first glance, it seemed to be obvious that it must have been a
> threading issue, which could easily be solved by adding volatile to the
> stored fooBarService field.
>
> However, I was more than confused seeing
> that javax.servlet.GenericServlet#config uses the same approach by simply
> storing the ServletConfig into a field and reading it later on without any
> locking etc.
>
> I quickly scanned through the Servlet specs and Catalina codes, but cannot
> really locate any explicit reference to thread-safety of
> javax.servlet.GenericServlet#getServletConfig, that is where the question
> originates.
>
> I would by much obliged if you had any inputs, ideas regarding this, or on
> the approach one could take to confirm it on his/her own.
In theory, it is a problem.
In practise, I'd be surprised if anyone was ever troubled by it.
Since it appears you did hit this issue, open a bug report and we'll get
Tomcat's implementation fixed.
Mark
>
> Thanks,
> Peter
>
>
> On Mon, Nov 28, 2016 at 6:45 PM, Christopher Schultz <
> chris@christopherschultz.net> wrote:
>
> P�ter,
>
> On 11/28/16 11:01 AM, P�ter Gergely Horv�th wrote:
>>>> Thank you very much for your feedback, but I think there is a
>>>> slight misunderstanding here: I know the order in which a servlet
>>>> is initialized and put into service, and this question is not
>>>> related to that. It's related to the _visibility_ of the field.
>>>>
>>>> The question is about the thread safety of the field access: if
>>>> you carefully check the details of
>>>> javax.servlet.GenericServlet#config and compare the implementation
>>>> to the sample "NoVisibility" from the book Java Concurrency in
>>>> Practice, then it is seems to follow the same pattern, which is
>>>> "not thread-safe because the value field is accessed from both get
>>>> and set without synchronization. Among other hazards, it is
>>>> susceptible to stale values: if one thread calls set, other threads
>>>> calling get may or may not see that update." [1].
>>>>
>>>> Personally, I would like to understand why this implementation is
>>>> not (if not) susceptible to the stale values phenomenon [2]; there
>>>> might be someone, who can point me to the right direction.
>
> I think you are correct that there is a theoretical multi-threaded
> race-condition, here. The container may initialize the servlet in one
> thread and service e.g. the first request in another thread, and the
> ServletContext reference might not be written to main memory and then
> read-back by the request-processing thread.
>
> Potential fixes for this would be either to define the ServletContext
> member to be volatile or to use synchronized methods.'
>
> Adding synchronization to the init() method would not be a problem at
> all: there is very little monitor contention at that stage and the
> container would only expect to call that method a single time. Adding
> synchronization to the getServletContext method, though, might
> represent a reduction in performance, since getServletContext gets
> called many times during the lifetime of a Servlet, and from many thread
> s.
>
> Likewise, marking the ServletContext member as "volatile" would
> necessarily require a slow main-memory read every time
> getServletContext is called.
>
> I suspect simple analysis above is the reason for no multithreaded
> protection being placed on the ServletContext member in question.
>
> P�ter, is this an academic discussion, or have you in fact seen a case
> where a servlet has been initialized and yet the first thread to
> process a request receives a null value when calling getServletContext?
>
> -chris
>
>>>> On Mon, Nov 28, 2016 at 6:08 AM, andreas <an...@junius.info>
>>>> wrote:
>>>>
>>>>> ---- On Fri, 25 Nov 2016 23:02:00 +0930 P�ter Gergely Horv�th
>>>>> wrote ----
>>>>>> Hi All,
>>>>>>
>>>>>> I am wondering why calling
>>>>>> javax.servlet.Servlet#getServletConfig() is thread safe: if you
>>>>>> check the implementation in javax.servlet.GenericServlet from
>>>>>> servlet API 3.0.1, you see the
>>>>> following:
>>>>>>
>>>>>> package javax.servlet;
>>>>>>
>>>>>> // lines omitted
>>>>>>
>>>>>> public abstract class GenericServlet implements Servlet,
>>>>>> ServletConfig, java.io.Serializable { // lines omitted
>>>>>>
>>>>>> private transient ServletConfig config;
>>>>>>
>>>>>> // lines omitted
>>>>>>
>>>>>> public void init(ServletConfig config) throws ServletException
>>>>>> { this.config = config; this.init(); }
>>>>>>
>>>>>> // lines omitted
>>>>>>
>>>>>> public ServletConfig getServletConfig() { return config; } //
>>>>>> lines omitted }
>>>>>>
>>>>>>
>>>>>> The field config is written in init(ServletConfig) without any
>>>>>> synchronization, locking or config being volatile, while
>>>>> getServletConfig()
>>>>>> could be called anytime from any later worker thread of the
>>>>>> servlet container. I took a quick look into Tomcat/Catalina
>>>>>> code base, however I see no indication of the servlet container
>>>>>> performing any magic, which would guarantee thread safe access
>>>>>> to field config through getServletConfig() from e.g.
>>>>>> javax.servlet.GenericServlet#service(ServletRequest,
>>>>>> ServletResponse) and the corresponding methods in
>>>>>> javax.servlet.http.HttpServlet.
>>>>>>
>>>>>> Am I missing something here? What will guarantee that if Thread
>>>>>> A is used to init(ServletConfig) then Thread B calling
>>>>>> getServletConfig() will see the correct state of the field
>>>>>> config (Java "happens-before"), and not
>>>>> e.g.
>>>>>> null?
>>>>>>
>>>>>> I am asking this because I have seen some legacy code, where a
>>>>>> servlet stored something into a field inside the init() method,
>>>>>> which was then
>>>>> used
>>>>>> from within a javax.servlet.http.HttpServlet#doGet or
>>>>>> javax.servlet.http.HttpServlet#doPost method, without
>>>>>> synchronization of any kind like this:
>>>>>>
>>>>>> public class FoobarServlet extends HttpServlet {
>>>>>>
>>>>>> private FoobarService foobarService;
>>>>>>
>>>>>> @Override public void init() throws ServletException {
>>>>>> this.foobarService = // ... acquire foobarService } protected
>>>>>> void doGet(HttpServletRequest request, HttpServletResponse
>>>>>> response) throws ServletException, IOException { List<Foobar>
>>>>>> foobars = this.foobarService.getFoobars(); // read the field
>>>>>> WITHOUT synchronization // ... } // ... Is this approach
>>>>>> (having no synchronization, locking or the field being
>>>>>> volatile) correct? I assumed it is not, seeing something like
>>>>>> that in the servlet API is quite confusing.
>>>>>>
>>>>>>
>>>>>> What do you think?
>>>>>>
>>>>>> Thanks, Peter
>>>>>
>>>>>
>>>>> A Servlet will process requests only if it is fully initialized,
>>>>> i.e. init has been executed. The init method gets called only
>>>>> once and the servlet config won't change afterwards. I don't
>>>>> think there is need for synchronization. The same is probably
>>>>> valid for your own objects. Problems arise when individual
>>>>> requests change the state of these objects.
>>>>>
>>>>> Andy
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>>
>>>>>
> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
>>>>> For additional commands, e-mail: users-help@tomcat.apache.org
>>>>>
>>>>>
>>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: users-help@tomcat.apache.org
>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Thread-safety of javax.servlet.Servlet#getServletConfig()
Posted by Péter Gergely Horváth <pe...@gmail.com>.
Hi Chris,
Thanks your four input: this question is somewhere in-between... :)
We have *definitely* seen cases, where a piece of code like the one below
sometimes (a couple of times from tens of thousands of successfully
serviced requests) found the stored field to be null - with a
NullPointerException being thrown on the first access of the fooBarService
field.
@WebServlet("/open")
public class FooBarServlet extends HttpServlet {
private FooBarService fooBarService;
@Override
public void init() throws ServletException {
try {
ApplicationContext applicationContext =
(ApplicationContext)
getServletContext().getAttribute("springContext");
fooBarService =
applicationContext.getBean("fooBarService",
FooBarService.class);
} catch (Exception ex) {
// wrap any exceptions to ServletException so as to comply with
Servlet contract
throw new ServletException("Initialization failed with
exception", ex);
}
}
protected void doGet(HttpServletRequest request, HttpServletResponse
response) throws ServletException, IOException {
processRequest(request, response);
}
protected void doPost(HttpServletRequest request, HttpServletResponse
response) throws ServletException, IOException {
processRequest(request, response);
}
private void processRequest(HttpServletRequest request,
HttpServletResponse response) throws IOException, ServletException {
List<FooBar> = fooBarService.getFooBars(); /// we have seen NPEs
thrown from here
/// ...
}
For the first glance, it seemed to be obvious that it must have been a
threading issue, which could easily be solved by adding volatile to the
stored fooBarService field.
However, I was more than confused seeing
that javax.servlet.GenericServlet#config uses the same approach by simply
storing the ServletConfig into a field and reading it later on without any
locking etc.
I quickly scanned through the Servlet specs and Catalina codes, but cannot
really locate any explicit reference to thread-safety of
javax.servlet.GenericServlet#getServletConfig, that is where the question
originates.
I would by much obliged if you had any inputs, ideas regarding this, or on
the approach one could take to confirm it on his/her own.
Thanks,
Peter
On Mon, Nov 28, 2016 at 6:45 PM, Christopher Schultz <
chris@christopherschultz.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Péter,
>
> On 11/28/16 11:01 AM, Péter Gergely Horváth wrote:
> > Thank you very much for your feedback, but I think there is a
> > slight misunderstanding here: I know the order in which a servlet
> > is initialized and put into service, and this question is not
> > related to that. It's related to the _visibility_ of the field.
> >
> > The question is about the thread safety of the field access: if
> > you carefully check the details of
> > javax.servlet.GenericServlet#config and compare the implementation
> > to the sample "NoVisibility" from the book Java Concurrency in
> > Practice, then it is seems to follow the same pattern, which is
> > "not thread-safe because the value field is accessed from both get
> > and set without synchronization. Among other hazards, it is
> > susceptible to stale values: if one thread calls set, other threads
> > calling get may or may not see that update." [1].
> >
> > Personally, I would like to understand why this implementation is
> > not (if not) susceptible to the stale values phenomenon [2]; there
> > might be someone, who can point me to the right direction.
>
> I think you are correct that there is a theoretical multi-threaded
> race-condition, here. The container may initialize the servlet in one
> thread and service e.g. the first request in another thread, and the
> ServletContext reference might not be written to main memory and then
> read-back by the request-processing thread.
>
> Potential fixes for this would be either to define the ServletContext
> member to be volatile or to use synchronized methods.'
>
> Adding synchronization to the init() method would not be a problem at
> all: there is very little monitor contention at that stage and the
> container would only expect to call that method a single time. Adding
> synchronization to the getServletContext method, though, might
> represent a reduction in performance, since getServletContext gets
> called many times during the lifetime of a Servlet, and from many thread
> s.
>
> Likewise, marking the ServletContext member as "volatile" would
> necessarily require a slow main-memory read every time
> getServletContext is called.
>
> I suspect simple analysis above is the reason for no multithreaded
> protection being placed on the ServletContext member in question.
>
> Péter, is this an academic discussion, or have you in fact seen a case
> where a servlet has been initialized and yet the first thread to
> process a request receives a null value when calling getServletContext?
>
> - -chris
>
> > On Mon, Nov 28, 2016 at 6:08 AM, andreas <an...@junius.info>
> > wrote:
> >
> >> ---- On Fri, 25 Nov 2016 23:02:00 +0930 Péter Gergely Horváth
> >> wrote ----
> >>> Hi All,
> >>>
> >>> I am wondering why calling
> >>> javax.servlet.Servlet#getServletConfig() is thread safe: if you
> >>> check the implementation in javax.servlet.GenericServlet from
> >>> servlet API 3.0.1, you see the
> >> following:
> >>>
> >>> package javax.servlet;
> >>>
> >>> // lines omitted
> >>>
> >>> public abstract class GenericServlet implements Servlet,
> >>> ServletConfig, java.io.Serializable { // lines omitted
> >>>
> >>> private transient ServletConfig config;
> >>>
> >>> // lines omitted
> >>>
> >>> public void init(ServletConfig config) throws ServletException
> >>> { this.config = config; this.init(); }
> >>>
> >>> // lines omitted
> >>>
> >>> public ServletConfig getServletConfig() { return config; } //
> >>> lines omitted }
> >>>
> >>>
> >>> The field config is written in init(ServletConfig) without any
> >>> synchronization, locking or config being volatile, while
> >> getServletConfig()
> >>> could be called anytime from any later worker thread of the
> >>> servlet container. I took a quick look into Tomcat/Catalina
> >>> code base, however I see no indication of the servlet container
> >>> performing any magic, which would guarantee thread safe access
> >>> to field config through getServletConfig() from e.g.
> >>> javax.servlet.GenericServlet#service(ServletRequest,
> >>> ServletResponse) and the corresponding methods in
> >>> javax.servlet.http.HttpServlet.
> >>>
> >>> Am I missing something here? What will guarantee that if Thread
> >>> A is used to init(ServletConfig) then Thread B calling
> >>> getServletConfig() will see the correct state of the field
> >>> config (Java "happens-before"), and not
> >> e.g.
> >>> null?
> >>>
> >>> I am asking this because I have seen some legacy code, where a
> >>> servlet stored something into a field inside the init() method,
> >>> which was then
> >> used
> >>> from within a javax.servlet.http.HttpServlet#doGet or
> >>> javax.servlet.http.HttpServlet#doPost method, without
> >>> synchronization of any kind like this:
> >>>
> >>> public class FoobarServlet extends HttpServlet {
> >>>
> >>> private FoobarService foobarService;
> >>>
> >>> @Override public void init() throws ServletException {
> >>> this.foobarService = // ... acquire foobarService } protected
> >>> void doGet(HttpServletRequest request, HttpServletResponse
> >>> response) throws ServletException, IOException { List<Foobar>
> >>> foobars = this.foobarService.getFoobars(); // read the field
> >>> WITHOUT synchronization // ... } // ... Is this approach
> >>> (having no synchronization, locking or the field being
> >>> volatile) correct? I assumed it is not, seeing something like
> >>> that in the servlet API is quite confusing.
> >>>
> >>>
> >>> What do you think?
> >>>
> >>> Thanks, Peter
> >>
> >>
> >> A Servlet will process requests only if it is fully initialized,
> >> i.e. init has been executed. The init method gets called only
> >> once and the servlet config won't change afterwards. I don't
> >> think there is need for synchronization. The same is probably
> >> valid for your own objects. Problems arise when individual
> >> requests change the state of these objects.
> >>
> >> Andy
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >> ---------------------------------------------------------------------
> >>
> >>
> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
> >> For additional commands, e-mail: users-help@tomcat.apache.org
> >>
> >>
> >
> -----BEGIN PGP SIGNATURE-----
> Comment: GPGTools - http://gpgtools.org
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iQIcBAEBCAAGBQJYPG0dAAoJEBzwKT+lPKRYkLUP/3JrIPCjIsmw4o71nmZUeN4r
> 0rt1ccflgjFUmpff5Pt9kHWJR60tga+sFT0jQ7LEvggbua1SoXV4y4MDINgEQ/Uu
> ek0aiDRR0n8xXxUWcx7P8Jjmn5yyOGUrFVw029dD55XT6ArYz7gaAUnowRKB6DR+
> RT84yO14TlRZx8DDLoxegmUpmXDpoHZyPKGGYWDhNepiU00X1YmFg/G3CiXJEjzd
> B3GBewhE8nh9GZsqfg7U1V9EBQI+8jeI/GxRTSnsUwWDKvH7Ja6ffiLtAS+3mnWD
> Ad2/wTbY7RT9ybT+thgvXkKJEKx+rVZrazb7QSgqq8Otv/aeMVLh16z2sxt6uuMy
> fffKRwNUL12cKr8KdNvUnb4PVStVF8fnRxAW1M90axzFvxeoLOJJZXtS6TBy+MjE
> eeMbjFV2wJw2y+66YWrvyCglyQU4c2ab5H0JuBINJ5oD1DSdTks0dW2rZvIm6pYp
> kCQTLSbZFjePksoNw6gxDo5XQW1SczWbTG2lYwEjfm3eeyHRnaRgiV4LPBPsx5Zy
> 7QFZJVGSFug2pgKTKErugoQs8z4gFCrryvebbGYgtvlsdJRk5Ldnl6uG0KabAEVU
> PedppbSTPPkE8Ym+/YTTpJ4Bo5NMjkoYySWnWpBLof5sJ1hkjkURc4cBiFw4vqZO
> K7WcPH4LDk5J5HzFiTIS
> =DLPe
> -----END PGP SIGNATURE-----
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@tomcat.apache.org
>
>
Re: Thread-safety of javax.servlet.Servlet#getServletConfig()
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
P�ter,
On 11/28/16 11:01 AM, P�ter Gergely Horv�th wrote:
> Thank you very much for your feedback, but I think there is a
> slight misunderstanding here: I know the order in which a servlet
> is initialized and put into service, and this question is not
> related to that. It's related to the _visibility_ of the field.
>
> The question is about the thread safety of the field access: if
> you carefully check the details of
> javax.servlet.GenericServlet#config and compare the implementation
> to the sample "NoVisibility" from the book Java Concurrency in
> Practice, then it is seems to follow the same pattern, which is
> "not thread-safe because the value field is accessed from both get
> and set without synchronization. Among other hazards, it is
> susceptible to stale values: if one thread calls set, other threads
> calling get may or may not see that update." [1].
>
> Personally, I would like to understand why this implementation is
> not (if not) susceptible to the stale values phenomenon [2]; there
> might be someone, who can point me to the right direction.
I think you are correct that there is a theoretical multi-threaded
race-condition, here. The container may initialize the servlet in one
thread and service e.g. the first request in another thread, and the
ServletContext reference might not be written to main memory and then
read-back by the request-processing thread.
Potential fixes for this would be either to define the ServletContext
member to be volatile or to use synchronized methods.'
Adding synchronization to the init() method would not be a problem at
all: there is very little monitor contention at that stage and the
container would only expect to call that method a single time. Adding
synchronization to the getServletContext method, though, might
represent a reduction in performance, since getServletContext gets
called many times during the lifetime of a Servlet, and from many thread
s.
Likewise, marking the ServletContext member as "volatile" would
necessarily require a slow main-memory read every time
getServletContext is called.
I suspect simple analysis above is the reason for no multithreaded
protection being placed on the ServletContext member in question.
P�ter, is this an academic discussion, or have you in fact seen a case
where a servlet has been initialized and yet the first thread to
process a request receives a null value when calling getServletContext?
- -chris
> On Mon, Nov 28, 2016 at 6:08 AM, andreas <an...@junius.info>
> wrote:
>
>> ---- On Fri, 25 Nov 2016 23:02:00 +0930 P�ter Gergely Horv�th
>> wrote ----
>>> Hi All,
>>>
>>> I am wondering why calling
>>> javax.servlet.Servlet#getServletConfig() is thread safe: if you
>>> check the implementation in javax.servlet.GenericServlet from
>>> servlet API 3.0.1, you see the
>> following:
>>>
>>> package javax.servlet;
>>>
>>> // lines omitted
>>>
>>> public abstract class GenericServlet implements Servlet,
>>> ServletConfig, java.io.Serializable { // lines omitted
>>>
>>> private transient ServletConfig config;
>>>
>>> // lines omitted
>>>
>>> public void init(ServletConfig config) throws ServletException
>>> { this.config = config; this.init(); }
>>>
>>> // lines omitted
>>>
>>> public ServletConfig getServletConfig() { return config; } //
>>> lines omitted }
>>>
>>>
>>> The field config is written in init(ServletConfig) without any
>>> synchronization, locking or config being volatile, while
>> getServletConfig()
>>> could be called anytime from any later worker thread of the
>>> servlet container. I took a quick look into Tomcat/Catalina
>>> code base, however I see no indication of the servlet container
>>> performing any magic, which would guarantee thread safe access
>>> to field config through getServletConfig() from e.g.
>>> javax.servlet.GenericServlet#service(ServletRequest,
>>> ServletResponse) and the corresponding methods in
>>> javax.servlet.http.HttpServlet.
>>>
>>> Am I missing something here? What will guarantee that if Thread
>>> A is used to init(ServletConfig) then Thread B calling
>>> getServletConfig() will see the correct state of the field
>>> config (Java "happens-before"), and not
>> e.g.
>>> null?
>>>
>>> I am asking this because I have seen some legacy code, where a
>>> servlet stored something into a field inside the init() method,
>>> which was then
>> used
>>> from within a javax.servlet.http.HttpServlet#doGet or
>>> javax.servlet.http.HttpServlet#doPost method, without
>>> synchronization of any kind like this:
>>>
>>> public class FoobarServlet extends HttpServlet {
>>>
>>> private FoobarService foobarService;
>>>
>>> @Override public void init() throws ServletException {
>>> this.foobarService = // ... acquire foobarService } protected
>>> void doGet(HttpServletRequest request, HttpServletResponse
>>> response) throws ServletException, IOException { List<Foobar>
>>> foobars = this.foobarService.getFoobars(); // read the field
>>> WITHOUT synchronization // ... } // ... Is this approach
>>> (having no synchronization, locking or the field being
>>> volatile) correct? I assumed it is not, seeing something like
>>> that in the servlet API is quite confusing.
>>>
>>>
>>> What do you think?
>>>
>>> Thanks, Peter
>>
>>
>> A Servlet will process requests only if it is fully initialized,
>> i.e. init has been executed. The init method gets called only
>> once and the servlet config won't change afterwards. I don't
>> think there is need for synchronization. The same is probably
>> valid for your own objects. Problems arise when individual
>> requests change the state of these objects.
>>
>> Andy
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> ---------------------------------------------------------------------
>>
>>
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: users-help@tomcat.apache.org
>>
>>
>
-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBCAAGBQJYPG0dAAoJEBzwKT+lPKRYkLUP/3JrIPCjIsmw4o71nmZUeN4r
0rt1ccflgjFUmpff5Pt9kHWJR60tga+sFT0jQ7LEvggbua1SoXV4y4MDINgEQ/Uu
ek0aiDRR0n8xXxUWcx7P8Jjmn5yyOGUrFVw029dD55XT6ArYz7gaAUnowRKB6DR+
RT84yO14TlRZx8DDLoxegmUpmXDpoHZyPKGGYWDhNepiU00X1YmFg/G3CiXJEjzd
B3GBewhE8nh9GZsqfg7U1V9EBQI+8jeI/GxRTSnsUwWDKvH7Ja6ffiLtAS+3mnWD
Ad2/wTbY7RT9ybT+thgvXkKJEKx+rVZrazb7QSgqq8Otv/aeMVLh16z2sxt6uuMy
fffKRwNUL12cKr8KdNvUnb4PVStVF8fnRxAW1M90axzFvxeoLOJJZXtS6TBy+MjE
eeMbjFV2wJw2y+66YWrvyCglyQU4c2ab5H0JuBINJ5oD1DSdTks0dW2rZvIm6pYp
kCQTLSbZFjePksoNw6gxDo5XQW1SczWbTG2lYwEjfm3eeyHRnaRgiV4LPBPsx5Zy
7QFZJVGSFug2pgKTKErugoQs8z4gFCrryvebbGYgtvlsdJRk5Ldnl6uG0KabAEVU
PedppbSTPPkE8Ym+/YTTpJ4Bo5NMjkoYySWnWpBLof5sJ1hkjkURc4cBiFw4vqZO
K7WcPH4LDk5J5HzFiTIS
=DLPe
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: Thread-safety of javax.servlet.Servlet#getServletConfig()
Posted by Péter Gergely Horváth <pe...@gmail.com>.
Hi Andy,
Thank you very much for your feedback, but I think there is a slight
misunderstanding here: I know the order in which a servlet is initialized
and put into service, and this question is not related to that. It's
related to the _visibility_ of the field.
The question is about the thread safety of the field access: if you
carefully check the details of javax.servlet.GenericServlet#config and
compare the implementation to the sample "NoVisibility" from the book Java
Concurrency in Practice, then it is seems to follow the same pattern, which
is "not thread-safe because the value field is accessed from both get and
set without synchronization. Among other hazards, it is susceptible to
stale values: if one thread calls set, other threads calling get may or may
not see that update." [1].
Personally, I would like to understand why this implementation is not (if
not) susceptible to the stale values phenomenon [2]; there might be
someone, who can point me to the right direction.
Thanks,
Peter
[1] Java Concurrency in Practice: Chapter 3. Sharing Objects 3.1 Visibility
[2]
https://www.javacodegeeks.com/2014/08/java-concurrency-tutorial-visibility-between-threads.html
On Mon, Nov 28, 2016 at 6:08 AM, andreas <an...@junius.info> wrote:
> ---- On Fri, 25 Nov 2016 23:02:00 +0930 Péter Gergely Horváth wrote ----
> >Hi All,
> >
> >I am wondering why calling javax.servlet.Servlet#getServletConfig() is
> >thread safe: if you check the implementation in
> > javax.servlet.GenericServlet from servlet API 3.0.1, you see the
> following:
> >
> >package javax.servlet;
> >
> >// lines omitted
> >
> >public abstract class GenericServlet
> > implements Servlet, ServletConfig, java.io.Serializable
> >{
> > // lines omitted
> >
> > private transient ServletConfig config;
> >
> > // lines omitted
> >
> > public void init(ServletConfig config) throws ServletException {
> > this.config = config;
> > this.init();
> > }
> >
> > // lines omitted
> >
> > public ServletConfig getServletConfig() {
> > return config;
> > }
> > // lines omitted
> >}
> >
> >
> >The field config is written in init(ServletConfig) without any
> >synchronization, locking or config being volatile, while
> getServletConfig()
> >could be called anytime from any later worker thread of the servlet
> >container. I took a quick look into Tomcat/Catalina code base, however I
> >see no indication of the servlet container performing any magic, which
> >would guarantee thread safe access to field config through
> >getServletConfig() from e.g.
> >javax.servlet.GenericServlet#service(ServletRequest, ServletResponse) and
> >the corresponding methods in javax.servlet.http.HttpServlet.
> >
> >Am I missing something here? What will guarantee that if Thread A is used
> >to init(ServletConfig) then Thread B calling getServletConfig() will see
> >the correct state of the field config (Java "happens-before"), and not
> e.g.
> >null?
> >
> >I am asking this because I have seen some legacy code, where a servlet
> >stored something into a field inside the init() method, which was then
> used
> >from within a javax.servlet.http.HttpServlet#doGet or
> >javax.servlet.http.HttpServlet#doPost method, without synchronization of
> >any kind like this:
> >
> >public class FoobarServlet extends HttpServlet {
> >
> > private FoobarService foobarService;
> >
> > @Override
> > public void init() throws ServletException {
> >this.foobarService = // ... acquire foobarService
> > }
> > protected void doGet(HttpServletRequest request, HttpServletResponse
> >response) throws ServletException, IOException {
> > List<Foobar> foobars = this.foobarService.getFoobars(); // read the
> >field WITHOUT synchronization
> > // ...
> > }
> > // ...
> >Is this approach (having no synchronization, locking or the field being
> >volatile) correct? I assumed it is not, seeing something like that in the
> >servlet API is quite confusing.
> >
> >
> >What do you think?
> >
> >Thanks,
> >Peter
>
>
> A Servlet will process requests only if it is fully initialized, i.e. init
> has been executed. The init method gets called only once and the servlet
> config won't change afterwards. I don't think there is need for
> synchronization. The same is probably valid for your own objects. Problems
> arise when individual requests change the state of these objects.
>
> Andy
>
>
>
>
>
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@tomcat.apache.org
>
>
Re: Thread-safety of javax.servlet.Servlet#getServletConfig()
Posted by andreas <an...@junius.info>.
---- On Fri, 25 Nov 2016 23:02:00 +0930 Péter Gergely Horváth wrote ----
>Hi All,
>
>I am wondering why calling javax.servlet.Servlet#getServletConfig() is
>thread safe: if you check the implementation in
> javax.servlet.GenericServlet from servlet API 3.0.1, you see the following:
>
>package javax.servlet;
>
>// lines omitted
>
>public abstract class GenericServlet
> implements Servlet, ServletConfig, java.io.Serializable
>{
> // lines omitted
>
> private transient ServletConfig config;
>
> // lines omitted
>
> public void init(ServletConfig config) throws ServletException {
> this.config = config;
> this.init();
> }
>
> // lines omitted
>
> public ServletConfig getServletConfig() {
> return config;
> }
> // lines omitted
>}
>
>
>The field config is written in init(ServletConfig) without any
>synchronization, locking or config being volatile, while getServletConfig()
>could be called anytime from any later worker thread of the servlet
>container. I took a quick look into Tomcat/Catalina code base, however I
>see no indication of the servlet container performing any magic, which
>would guarantee thread safe access to field config through
>getServletConfig() from e.g.
>javax.servlet.GenericServlet#service(ServletRequest, ServletResponse) and
>the corresponding methods in javax.servlet.http.HttpServlet.
>
>Am I missing something here? What will guarantee that if Thread A is used
>to init(ServletConfig) then Thread B calling getServletConfig() will see
>the correct state of the field config (Java "happens-before"), and not e.g.
>null?
>
>I am asking this because I have seen some legacy code, where a servlet
>stored something into a field inside the init() method, which was then used
>from within a javax.servlet.http.HttpServlet#doGet or
>javax.servlet.http.HttpServlet#doPost method, without synchronization of
>any kind like this:
>
>public class FoobarServlet extends HttpServlet {
>
> private FoobarService foobarService;
>
> @Override
> public void init() throws ServletException {
>this.foobarService = // ... acquire foobarService
> }
> protected void doGet(HttpServletRequest request, HttpServletResponse
>response) throws ServletException, IOException {
> List<Foobar> foobars = this.foobarService.getFoobars(); // read the
>field WITHOUT synchronization
> // ...
> }
> // ...
>Is this approach (having no synchronization, locking or the field being
>volatile) correct? I assumed it is not, seeing something like that in the
>servlet API is quite confusing.
>
>
>What do you think?
>
>Thanks,
>Peter
A Servlet will process requests only if it is fully initialized, i.e. init has been executed. The init method gets called only once and the servlet config won't change afterwards. I don't think there is need for synchronization. The same is probably valid for your own objects. Problems arise when individual requests change the state of these objects.
Andy
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org