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 <
ch...@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 <andr...@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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to