Should EntityManagerFactory be closed at application shutdown?-CodePudding

I have a Java application that has a GUI made with Swing and that uses two databases interchangeably. One of the two databases is mongoDB and the other one is MySQL. Which database to use is chosen with a command line option. For the MySQL database I am also using Hibernate and JPA. The code I have looks like this:

import picocli.CommandLine;
import picocli.CommandLine.Command;
import picocli.CommandLine.Option;
import java.awt.EventQueue;
import java.util.concurrent.Callable;

import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import javax.persistence.Persistence;

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

@Command(mixinStandardHelpOptions = true)
public class App implements Callable<Void> {
    private static final Logger LOGGER = LogManager.getLogger(App.class);

    @Option(names = { "--database" }, description = "'mongo' or 'mysql'")
    private String databaseType = "mysql";

    public static void main(String[] args) {
        new CommandLine(new App()).execute(args);
    }

    @Override
    public Void call() throws Exception {

        EventQueue.invokeLater(() -> {

            switch (databaseType) {
            case "mysql":
                EntityManagerFactory emf;
                EntityManager entityManager;

                try {
                    emf = Persistence.createEntityManagerFactory("name");
                    entityManager = emf.createEntityManager();
                    // other stuff
                    
                } catch (Exception e) {
                    LOGGER.log(Level.ERROR, "MySQL Exception", e);
                }

                break;
            case "mongo":
                // mongo stuff, no EntityManagerFactory here
                break;

            default:
                LOGGER.log(Level.ERROR, "--database must be either 'mysql' or 'mongo'");
                System.exit(1);
            }
            //...
            try {
                View view = new View();
                view.setVisible(true);
            } catch (Exception e) {
                LOGGER.log(Level.ERROR, "Exception", e);
            }
        });
        return null;
    }

In mysql case I am creating an EntityManagerFactory and an EntityManager. The entityManager created here is passed as argument to the constructor of the repositories and used throughout the whole life of the application. I was wondering what is the best practice about closing the entityManager and the factory. Searching in the documentation I found this:

Closing an EntityManagerFactory should not be taken lightly. It is much better to keep a factory open for a long period of time than to repeatedly create and close new factories. Thus, most applications will never close the factory, or only close it when the application is exiting.

So I was wondering, what is the difference between closing the factory and entity manager at application shutdown and not closing it? Also in my case I'm declaring emf and entityManager inside the mysql case since are not required for mongodb. In order to close them at application shutdown what should I do? I found something about Runtime.getRuntime().addShutdownHook(). I tried using it like the code below, but it seems like it is not working.

try {
    emf = Persistence.createEntityManagerFactory("name");
    entityManager = emf.createEntityManager();
    Thread closeHook = new Thread(() -> {
        if (emf != null) {
            entityManager.close();
            emf.close();
            LOGGER.log(Level.INFO, "Close entity manager and entity manager factory");
        }
    });
    Runtime.getRuntime().addShutdownHook(closeHook);
    // other stuff
                        
} catch (Exception e) {
    LOGGER.log(Level.ERROR, "MySQL Exception", e);
}

CodePudding user response：

Short answer, yes, it should be closed. And the reason can be found at this answer:

The JVM will release all active resources upon termination; however, this does not ensure that the other end will free the resource too, so explicitly closing resources is in every programmer's best interest.

So in my case, it is true that the EntityManager and factory are closed at application shutdown, but this does not ensure that they are properly dealt with on the other end. I didn't mention it in my question, but in fact the same thing holds true for the Mongo Client as well (see this answer):

If you ever re-deploy your web application without first restarting your application server, you must ensure that the MongoClient is closed when your web application is shutdown.

About the implementation I made an interface that I called DBInitializer. I instantiated an object of type MongoInitializer or MySQLInitializer (both implementing DBInitializer) inside the main method. See code for more clarity.

DBInitializer:

public interface DBInitializer {
    public void startDbConnection();
    public void closeDbConnection();
}

MySQLInitializer:

import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import javax.persistence.Persistence;

public class MySQLInitializer implements DBInitializer {
    private EntityManagerFactory emf;
    private EntityManager entityManager;
    private final Logger logger = LogManager.getLogger(MySQLInitializer.class);

    @Override
    public void startDbConnection() {
        try {
            emf = Persistence.createEntityManagerFactory("name");
            entityManager = emf.createEntityManager();
            // other stuff
        } catch (Exception e) {
            logger.log(Level.ERROR, "MySQL Exception", e);
        }
    }
    
    @Override
    public void closeDbConnection() {
        if (emf != null) {
            entityManager.close();
            emf.close();
        }
    }
}

MongoInitializer:

import com.mongodb.MongoClient;
import com.mongodb.ServerAddress;

public class MongoInitializer implements DBInitializer {
    
    private MongoClient client;
    private final Logger logger = LogManager.getLogger(MongoInitializer.class);
    
    @Override
    public void startDbConnection() {
        try {
            client = new MongoClient(new ServerAddress("localhost", 27017));
            // other stuff
        } catch (Exception e) {
            logger.log(Level.ERROR, "Mongo Exception", e);
        }
    }

    @Override
    public void closeDbConnection() {
        client.close();
    }
}

App:

import java.awt.EventQueue;
import java.util.concurrent.Callable;

import DBInitializer;
import MongoInitializer;
import MySQLInitializer;
import View;

import picocli.CommandLine;
import picocli.CommandLine.Command;
import picocli.CommandLine.Option;

@Command(mixinStandardHelpOptions = true)
public class App implements Callable<Void> {

    private static final Logger LOGGER = LogManager.getLogger(App.class);

    @Option(names = { "--database" }, description = "Either 'mongo' or 'mysql'")
    private String databaseType = "mysql";

    public static void main(String[] args) {
        new CommandLine(new App()).execute(args);
    }

    DBInitializer dBInitializer;

    @Override
    public Void call() throws Exception {
        EventQueue.invokeLater(() -> {
            try {
                switch (databaseType) {
                case "mysql":
                    dBInitializer = new MySQLInitializer();
                    break;
                case "mongo":
                    dBInitializer = new MongoInitializer();
                    break;
                default:
                    LOGGER.log(Level.ERROR, "--database must be either 'mysql' or 'mongo'");
                    System.exit(1);
                }
                dBInitializer.startDbConnection();
                // other stuff
                View view = new View();
                view.setVisible(true);
                
            } catch (Exception e) {
                LOGGER.log(Level.ERROR, "Exception", e);
            }
        });

        Runtime.getRuntime().addShutdownHook(new Thread() {
            @Override
            public void run() {
                dBInitializer.closeDbConnection();
            }
        });
        return null;
    }
}

CodePudding user response：

what is the difference between closing the factory and entity manager at application shutdown and not closing it?

A potential resource leak (e.g. an unclosed connection pool) vs lack thereof. Also:

I tried using it like the code below, but it seems like it is not working.

Why not use a try-with-resources statement?