Execute startup method asynchronously The goal is to call startManager on application start so that the manager is initialized in background without blocking application main thread.

Is it a safe implementation? Any recommendation or best practice? (use an Executor, use a synchronized block, ...)

public class FileConverter
{
    /** The Constant logger. */
    private static final Logger logger = LoggerFactory.getLogger(FileConverter.class);

    private static RunnableFuture&lt;OfficeManager&gt; managerFuture;

    /**
     * Start manager.
     */
    public static synchronized void startManager()
    {
        if(managerFuture == null)
        {
            Callable&lt;OfficeManager&gt; callable = new Callable&lt;OfficeManager&gt;()
            {
                @Override
                public OfficeManager call() throws Exception
                {
                    OfficeManagerConfiguration configuration = new OfficeManagerConfiguration();
                    OfficeManager officeManager = configuration.buildOfficeManager();

                    officeManager.start();

                    logger.info("officeManager started");

                    return officeManager;
                }
            };

            managerFuture = new FutureTask&lt;OfficeManager&gt;(callable);
            new Thread(managerFuture).start();
        }
    }

    /**
     * Stop manager.
     */
    public static synchronized void stopManager()
    {
        if(managerFuture != null)
        {
            managerFuture.cancel(true);

            try
            {
                OfficeManager officeManager = managerFuture.get();
                if(officeManager.isRunning())
                {
                    officeManager.stop();
                }
            }
            catch(CancellationException e)
            {
                // ignore
            }
            catch(InterruptedException e)
            {
                Thread.currentThread().interrupt();
            }
            catch(ExecutionException e)
            {
                logger.warn(e.getMessage(), e);
            }
        }
    }

    /**
     * Convert.
     *
     * @param file the file
     * @param format the format
     * @return the file
     * @throws Exception the exception
     */
    public static File convert(File file, String format) throws Exception
    {
        OfficeManager officeManager = managerFuture.get();
        if(!officeManager.isRunning())
        {
            throw new IllegalStateException("officeManager is not running!");
        }

        File tempFile = File.createTempFile(FileConverter.class.getSimpleName() + "-", "." + format);
        try
        {
            if(file.getName().endsWith("." + format))
            {
                FileUtils.copyFile(file, tempFile);
            }
            else
            {
                OfficeDocumentConverter converter = new OfficeDocumentConverter(officeManager);
                converter.convert(file, tempFile);
            }

            return tempFile;
        }
        catch(Exception e)
        {
            tempFile.delete();
            throw e;
        }
    }
}




Reply to @EricStein


Right. Adding synchronyzed to stopManager() should serialize invocations.
Right. It can cause an unwanted restart.
Right. I misunderstood the InterruptedException being generated in manager thread not by future.get() itself.
This is server-side code: every time I recompile the server, it takes about 30s to startup. This implementation reduces startup time to 25s. I agree it's not worth on production, but it's a time saver on development.


before accepting your answer, I'd like to know if new Thread(managerFuture).start(); is a good way to start an asynchronous operation: it doesn't sound good to start something without preserving a reference...
There's a potential failure. If a thread invokes startManager() and then another thread immediately invokes stopManager(), stopManager() could see managerFuture as null and do nothing before startManager() gets a chance to populate it. The end result would be as if stopManager() was never called.

There's no reason for convert() to call startManager() if the manager gets started on application start. 

Don't ignore InterruptedException. At the very least call Thread.currentThread().interrupt(). There are many resources out there on how to correctly handle InterruptedException, including on StackOverflow.

I also question the premise that the manager needs to start up on another thread. You should only bother if this is client-side code and this is a demonstrated bottleneck on startup.

Updated Answer


It will, but synchronized is a blunt instrument. It usually locks for longer than you need, and it's a lock on the object itself. If somebody else used your class as a lock, none of your synchronized methods could run until it was released. If you insist on going this route, you should use a private static final RentrantLock and minimize the scope to what you really need to be locked.
No, it shouldn't, but it doesn't do anything. If you call startManager() on startup, then the init thread is already running. If convert() also calls it, convert() will wait for the init thread to release its lock, then find out that it's already been initialized. OR you can do lazy-load of the manager in convert(), but then you have to wait for it to spin up the first time, which doesn't seem optimal if it takes 5 seconds.
Ok.
My point is that introducing this logic to save yourself 5 seconds on application start-up in dev is a bad tradeoff. Performance tuning is fragile and usually difficult to understand. Don't do it unless you need to. Junior programmers will get confused and muck it up by accident.
I don't think you need a reference to the thread. You have the Future .. that's what you care about. The only operation you want to perform on the thread after you start it is to interrupt it, which you can do from the future. If you need to hold onto the thread for some reason, by all means do so, but if you don't, there's no point in keeping a pointer to it.