[FIXED] Making Java Servlet Thread safe

Issue

I've been getting mixed up responses on the execution of my API from webpage.

Backtracked a little and found out, Angular was firing it the right way, but the API responded with mixed responses for concurrent requests.

Searched the internet found out Global variables in Java servlet may trigger the abnormal behavior as one thread might access the variable currently being processed by another variable.

So coming to the point I realized PrintWriter is creating a problem here. But what I'm experiencing a difficulty with is how to get out of the problem.

And also may be the current problem is not at all related to my conclusions. Please correct me wherever I've wrongly derived anything wrong.

@WebServlet(name = "WorkSpaceDetails", urlPatterns = {"/WorkSpaceDetails"})
public class WorkSpaceDetails extends HttpServlet {

    PrintWriter out;
    private static final long serialVersionUID = 1L;


    private void processRequest(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException, Exception {
        String wid = request.getParameter("id");

        try {

            id = Integer.parseInt(wid);
            response.setContentType("application/json");
            response.setCharacterEncoding("utf-8");
            out = response.getWriter();
            Connection conn = null;
            PreparedStatement prx = null;
            PreparedStatement pry = null;
            ResultSet set = null;

            ResultSetMetaData metaData;
            String query;
            JSONObject obj = new JSONObject();
            String WorkSpaceType;
            List<HashMap> completeData = new ArrayList<HashMap>();
            List<String> ColoumnNames = new ArrayList<String>();
            List<String> Data = new ArrayList<String>();
            try {
                query = "-";
                conn = boneCPConnectionPool.getConnection();
                prx = conn.prepareStatement(query);
                prx.setInt(1, id);
                set = prx.executeQuery();
                while (set.next()) {
                    WorkSpaceType = set.getString("WorkSpaceType");
                    System.out.println("WorkSpaceType:" + WorkSpaceType);
                    if (WorkSpaceType.equalsIgnoreCase("1")) {
                        System.out.println("" + 1);
                        query = "-";
                    } else {
                        System.out.println("" + 0);
                        query = "-";
                    }
                    conn = boneCPConnectionPool.getConnection();
                    pry = conn.prepareStatement(query);
                    pry.setInt(1, id);
                    set = pry.executeQuery();
                    metaData = set.getMetaData();
                    int count = metaData.getColumnCount();
                    for (int i = 1; i <= count; i++) {
                        System.out.println("" + metaData.getColumnName(i));
                        ColoumnNames.add(metaData.getColumnName(i));
                    }
                    while (set.next()) {
                        HashMap tmp = new HashMap();
                        for (int i = 0; i < ColoumnNames.size(); i++) {

                            String cname = ColoumnNames.get(i);

                            tmp.put(cname, set.getString(cname));
                            tmp.put("WorkSpaceType", WorkSpaceType);
                        }
                        System.out.println("" + tmp);
                        completeData.add(tmp);
                        generateJson(completeData);
                    }


                }

            } catch (Exception e) {
                e.printStackTrace();
                out.println(e);
            } finally {

                if (conn != null) {
                    try {
                        conn.close();
                    } catch (Exception e) {
                    }
                }
                if (set != null) {
                    try {
                        set.close();
                    } catch (SQLException ignore) {
                    }
                }
                if (prx != null) {
                    try {
                        prx.close();
                    } catch (SQLException ignore) {
                    }
                }
                if (pry != null) {
                    try {
                        pry.close();
                    } catch (SQLException ignore) {
                    }
                }

            }
        } catch (Exception e) {

            JSONObject obj = new JSONObject();
            obj.put("Error", "BLANK ID");
            System.out.println(obj.toString());
            e.printStackTrace();
        }
    }

    private void generateJson(List<HashMap> data) {
        JSONObject obj = new JSONObject();
        try {
            for (HashMap map : data) {

                Iterator it = map.entrySet().iterator();

                while (it.hasNext()) {

                    Map.Entry pair = (Map.Entry) it.next();
                    String key = (String) pair.getKey();

                    String value = (String) pair.getValue();
                    if (value == null) {
                        value = "blank";
                    }
                    obj.put(key, value);
                }
            }

            out.println(obj.toString());

        } catch (Exception e) {
            e.printStackTrace();
        }

    }

    @Override
    protected void doGet(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        out = response.getWriter();
        JSONObject obj = new JSONObject();
        try {
            obj.put("Error", "API Protected Get Request");
        } catch (JSONException ex) {
            Logger.getLogger(WorkSpaceDetails.class.getName()).log(Level.SEVERE, null, ex);
        }
        out.println(obj.toString());
        out.close();
        System.out.println("API Protected:Get Request");
    }

    @Override
    protected void doPost(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        out = response.getWriter();
        JSONObject obj = new JSONObject();
        try {
            processRequest(request, response);
        } catch (NumberFormatException ex) {
            try {
                obj.put("Error", "ID NULL");
            } catch (JSONException ex1) {
                Logger.getLogger(WorkSpaceDetails.class.getName()).log(Level.SEVERE, null, ex1);
            }
            out.println(obj.toString());
        } catch (Exception ex) {

            try {
                obj.put("Error", "TOKEN NULL");
            } catch (JSONException ex1) {
                Logger.getLogger(WorkSpaceDetails.class.getName()).log(Level.SEVERE, null, ex1);
            }
            out.println(obj.toString());
            out.close();

        }
    }

    @Override
    public String getServletInfo() {
        return "Servlet to get info of a Workspace";
    }

}

Solution

Instead of declaring PrintWriter out; as a class member, do it as a local variable inside processRequest(), like you're already doing in doGet() and doPost(). This way, the multiple threads that may access the servlet concurrently won't interfere with each other.

UPDATE

It seems that processRequest() is only called from doPost(), so I suggest you to:

• Get rid of PrintWriter out class member
• Declare a local PrintWriter out in doGet()
• Declare a local PrintWriter out in doPost()
• Pass out as a parameter to processRequest(), whose signature may be:

private void processRequest(
    HttpServletRequest request, HttpServletResponse response, PrintWriter out);

UPDATE

• Pass out as a parameter to generateJson()
• Pass out as a parameter to call made to 'generateJson' method at processRequest()

private void generateJson(List<HashMap> data, PrintWriter out);

generateJson(completeData, out);

Answered By - nandsito
Answer Checked By - Terry (JavaFixing Volunteer)