0

I've recently starting working on a java webapp (JSP / Servlet) that was developed by the internal developer of a company.

This app randomly doesn't return data, and inspecting the log I found some NullPointerExceptions related to the classes' member variable which holds the database connection. Following the stack trace it seems that a second thread closes the connection after it ended its task leaving the first thread without a connection.

By the needs of the company the app uses different databases, one which rules appdata, and others which contain data the app has to retrieve. So every class attached to the main servlet may connect to one or more databases depending on the task it has to accomplish.

I'm not familiar with JavaEE but giving a look at the database connection class, I see nothing which protect threads from conflicting each other.

Which is the correct way to handle such connections?

This is the code of the Database handler:

package it.metmi.mmasgis.utils;

import java.sql.*;
import java.util.ArrayList;
import java.util.HashMap;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

public class DBManager
{
    private String szDatabase;
    private String szUsername;
    private String szPassword;
    private String szError;
    private Connection db;
    private boolean bConnected;
    private Logger logger;

    public DBManager(String szDBName)
    {
        this(szDBName, "", "");
    }

    public DBManager(String szDBName, String szName, String szPass)
    {
        szDatabase = szDBName;
        szUsername = szName;
        szPassword = szPass;
        bConnected = false;
        szError = "";
        logger = LogManager.getFormatterLogger(DBManager.class.getName());
    }

    public boolean connect()
    {
        logger.entry();

        try {
            Class.forName("com.mysql.jdbc.Driver");

            if(!szDatabase.isEmpty())
            {
                String szCon = "jdbc:mysql://localhost/" + szDatabase;

                if(!szUsername.isEmpty())
                {
                    szCon += "?user=" + szUsername;

                    if(!szPassword.isEmpty())
                        szCon += "&password=" + szPassword;
                }

                db = DriverManager.getConnection(szCon);
                bConnected = true;
            } else {
                logger.error("No database name!!");
                System.exit(0);
            }
        } catch(SQLException | ClassNotFoundException e) {
            szError = e.getMessage();
            e.printStackTrace();
            logger.error("Can't connect: %s", e);
        }

        return logger.exit(bConnected);
    }

    public void disconnect()
    {
        logger.entry();

        try {
            db.close();
            bConnected = false;
        } catch(SQLException e) {
            e.printStackTrace();
            logger.error("Can't disconnect: %s", e);
        }

        logger.exit();
    }

    public boolean isConnected()
    {
        return bConnected;
    }

    public String getError()
    {
        return szError;
    }

    public ArrayList<HashMap<String,String>> query(String szQuery)
    {
        logger.entry(szQuery);

        ArrayList<HashMap<String,String>> aResults = new ArrayList<HashMap<String,String>>();

        int iCols = 0;
        try {
            Statement stmt = db.createStatement();

            logger.info("Query: %s", szQuery);
            ResultSet rs = stmt.executeQuery(szQuery);
            ResultSetMetaData rsmd = rs.getMetaData();
            iCols = rsmd.getColumnCount();

            while(rs.next())
            {
                HashMap<String,String> pv = new HashMap<String,String>();
                for(int i = 0; i < iCols; i++)
                {
                    String szCol = rsmd.getColumnLabel(i + 1);
                    String szVal = rs.getString(i + 1);
                    pv.put(szCol, szVal);
                }
                aResults.add(pv);
            }
            rs.close();
            stmt.close();
        } catch(SQLException e) {
            e.printStackTrace();
            szError = e.getMessage();
            logger.error("Error executing query: %s", e);
        }

        return logger.exit(aResults);
    }

    public boolean update(String szQuery)
    {
        logger.entry(szQuery);

        boolean bResult = false;

        try {
            Statement stmt = db.createStatement();

            logger.info("Query: %s", szQuery);
            stmt.executeUpdate(szQuery);

            bResult = true;
            stmt.close();
        } catch(SQLException e) {
            e.printStackTrace();
            szError = e.getMessage();
            bResult = false;
            logger.error("Error executing query: %s", e);
        }
        return logger.exit(bResult);
    }
}

The class Task which all the servlet classes are based on, is a simple abstract class:

package it.metmi.mmasgis.servlet;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

public abstract class Task
{
    public abstract void doTask(HttpServletRequest request, HttpServletResponse response);
}

The class which throws NullPointerExceptions it this one, during the invocation of db.disconnect(). This class is called rapidly via AJAX 4 or 5 times from the interface written in JS.

package it.metmi.mmasgis.servlet.params;

import it.metmi.mmasgis.servlet.Task;
import it.metmi.mmasgis.utils.Const;
import it.metmi.mmasgis.utils.DBManager;
import it.metmi.mmasgis.utils.Query;
import it.metmi.mmasgis.utils.Utility;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import java.io.IOException;
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.HashMap;

import com.google.gson.Gson;
import com.google.gson.GsonBuilder;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

public class ClassType extends Task
{
    private DBManager db = null;
    private Logger logger = LogManager.getFormatterLogger(ClassType.class.getName());

    @Override
    public void doTask(HttpServletRequest request, HttpServletResponse response)
    {
        logger.entry(request, response);

        String szCensimento = Utility.getParameter(request, "censimento");
        String szCategoria = Utility.getParameter(request, "category");
        ArrayList<HashMap<String,String>> aClasses = new ArrayList<HashMap<String,String>>();
        PrintWriter out = null;

        logger.debug("Census: %s", szCensimento);
        logger.debug("Category: %s", szCategoria);

        db = new DBManager(szCensimento, Const.DB_USER, Const.DB_PASS);

        if(db.connect())
        {
            String szQuery = String.format(Query.classes, szCategoria, szCategoria);
            aClasses = db.query(szQuery);

            db.disconnect();
        }

        try {
            out = response.getWriter();
            jsonEncode(aClasses, out);
        } catch(IOException e) {
            e.printStackTrace();
            logger.error("Failed to encode JSON: %s", e);
        }

        logger.exit();
    }

    private void jsonEncode(ArrayList<HashMap<String,String>> aData, PrintWriter out)
    {
        HashMap<String,Object> result = new HashMap<String,Object>();
        result.put("results", aData);
        result.put("success", true);

        Gson gson = new GsonBuilder().create();
        gson.toJson(result, out);
    }
}

If the webapp would use only one database, it could be rewritten as a Singleton, but in this way I have no idea on how to handle different connections for different databases. How can avoid these exceptions?

HelLViS69
  • 299
  • 1
  • 4
  • 15
  • Basically this is not the right decision to handle such cases manually. Why not to use some common practices, for example, to use Spring and Spring JDBC to handle all this stuff. You're doing to much work manually which you are not supposed to do. Take a look at this: https://spring.io/guides/gs/relational-data-access/ – Yuri Jun 12 '15 at 08:16
  • I just read the guide you linked. So do I need to rewrite the whole webapp to use Spring? Moreover, this app isn't based on Gradle, Maven or whatever.. – HelLViS69 Jun 12 '15 at 08:53
  • It's just on of the possible and popular approaches if you're planning that your application will be getting bigger. But if it's not then go ahead and read also this post, please. http://stackoverflow.com/questions/5003142/jsp-using-mvc-and-jdbc/5003701#5003701 – Yuri Jun 12 '15 at 08:58
  • Gradle, Maven - these are build tools, that help you. From my point of view, your app is small and you don't have much time to dive into spring (I'm recommend you to dive into it if you're planning to do web) then you could go with the link from my previous comment. – Yuri Jun 12 '15 at 09:01
  • Thanks for the advice. The fact that scares me is that this webapp is a little monster wrote by different hands during time. I'm trying to refactor all code because it's very ugly and redundant, but connection to databases is one of the main stuff I didn't touch until NPE appeared. I'll dive into Spring as I have time to spend, but for now I'd just want to fix this bad behaviour, so any help that doesn't let me rewrite all stuff from scratch is welcome – HelLViS69 Jun 12 '15 at 10:04

1 Answers1

0

The problem was that the connection object was declared as member. Moving the variable inside the methods resolved.

HelLViS69
  • 299
  • 1
  • 4
  • 15