dev@glassfish.java.net

Re: FindBugs errors - MSF_MUTABLE_SERVLET_FIELD - false positive?

From: Rama Pulavarthi <Rama.Pulavarthi_at_oracle.com>
Date: Wed, 20 Apr 2011 08:34:50 -0700

Hi,

>>>>> The description of MSF_MUTABLE_SERVLET_FIELD says:
>>>>>
>>>>> "A web server generally only creates one instance of servlet or
>>>>> jsp class (i.e., treats the class as a Singleton), and will have
>>>>> multiple threads invoke methods on that instance to service
>>>>> multiple simultaneous requests. Thus, having a mutable instance
>>>>> field generally creates race conditions."
>>>>>
>>>>> Would it be possible to make your field a *static* variable of
>>>>> your Servlet class instead of an instance variable and thereby
>>>>> signal to FindBugs that you take care of the fact that it will not
>>>>> create a race condition?
>>>>>
>>>> No, the value is associated with an instance and computed from
>>>> ServletConfig. I could have synchronized, But I feel its not
>>>> warranted for this case.
>>>
>>> volatile might be enough.
>>
>> If marking the field as "volatile" indeed were sufficient to get rid
>> of the FindBugs warning, that would be kind of absurd given the
>> original reasoning for the warning (see above).
> It was rather comment to "I could have synchronized" than the actual
> findbugs error, which seems knows nothing about Servlet's init(...)
> method.
>
I meant using volatile is not needed as well. I see a bug fix in
findbugs to allow for Servlet#init as in
http://code.google.com/p/findbugs/source/detail?r=5414&path=/trunk/findbugs/src/java/edu/umd/cs/findbugs/detect/MultithreadedInstanceAccess.java.

This seems to just check if the method name is "init", In our case we
are doing in doInit() called from init(). It does look like a bug in
FindBugs. I guess one way to keep findbugs happy is to move the code to
init().

thanks,
Rama Pulavarthi
> WBR,
> Alexey.
>