views:

222

answers:

2

I need to know if there are any issues with the below code as far as threading goes. I was always under the impression that so long as class level variables are not used threading is not an issue.

public class UploadReferralImage extends HttpServlet
{
    String CLASS_NAME = "UploadReferralImage";

public void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
   // Not handling Get, service must be invoked via Post.
}

public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
    String METHOD_NAME = "doPost";
    LogHelper.debug(PACKAGE_NAME, CLASS_NAME, METHOD_NAME, "Made it to the servlet");
    String reply = upload(request);
    response.setHeader("Content-Type", "text/xml");
    response.getWriter().write(reply);
    response.getWriter().flush();
    response.getWriter().close();
}

public String upload(HttpServletRequest request)
{

    String METHOD_NAME = "upload";
    LogHelper.debug(PACKAGE_NAME, CLASS_NAME, METHOD_NAME, "Inside upload");
    String replyMsg = "Unable to call ImageUpload";

    try
    {
        ObjectInputStream inputFromApplet = new ObjectInputStream(request.getInputStream());
        FileBean fBean = (FileBean) inputFromApplet.readObject();

        inputFromApplet.close();
        LogHelper.debug(PACKAGE_NAME, CLASS_NAME, METHOD_NAME, fBean.getFileName());

        replyMsg = doImageUpload(fBean);

        } 
        catch (IOException e)
    { 
        e.printStackTrace();
            replyMsg = "Exception :" + e.toString();

    } 
        catch (ClassNotFoundException e)
    {
        e.printStackTrace();
            replyMsg = "Exception :" + e.toString();
    }

    return replyMsg;
}

private String doImageUpload(FileBean fBean)
{
       //Save the file and return the response
       SaveCaseClientAgent saveCaseClientAgent = new SaveCaseClientAgent();
       saveCaseClientAgent.save(fBean);
}
+1  A: 

You are correct.

As long as you stay away from using class-level variables your Servlet will be thread safe.

To be safe, might as well make your class-level String final:

final String CLASS_NAME = "UploadReferralImage";
jjnguy
A: 

Usually there is only one instance of a servlet in servlet container that reentrantly serves multiple requests at a time. So using instance variables to pass thing from one method to another is a baad idea because of race conditions.

Best (and easiest) way to solve it is to write your own "handler" class that you will instantiate once per request and pass everything to it. In code it looks like that:

public void doGet(HttpServletRequest req, HttpServletResponse resp) {
   new MyHandler().doGet(req, resp);
}

Now in MyHandler reusing instance variables is as safe as it gets.

Marcin
So with the way I am doing it by passingpublic void doGet(HttpServletRequest req, HttpServletResponse resp) { upload(request);}could potentially cause a problem?
Knife-Action-Jesus
No. In your case there is no shared state between two different requests (call stacks are not shared between threads, references passed to methods are shared via stack). That said my 'trick' is still a cleaner, more widely used solution.
Marcin