3
\$\begingroup\$

I'm writing start/stop module for my application.

I have a class that manages application life cycle and I called it Work. Work uses WorkState class that contains 4 states the application can be in, that's:

  • INITIALIZING (state between NOT_RUNNING and RUNNING)
  • RUNNING
  • FINALIZING (state between RUNNING and NOT_RUNNING)
  • NOT_RUNNING

The class also contains some useful methods to manage states.

Work uses QueryExecutor. QueryExecutor is just a subclass of ThreadPoolExecutor with overriden method afterExecute() that changes application state after performing a start/stop task. (e.g. changing state from INITIALIZING to RUNNING after performing a start operation).

MY VISION:

When a thread invoke the start()/stop() methods, the code mustn't allow other thread to interfere the process until it's done. Also, there's no way to perform the stop action when the system is not started and vice versa.

MY CONCERNS:

  • Should I change locking system to "fair"?
  • Will it work smoothly with 2 threads? (There's no possibility that the start()/stop() can be accessed at the same time by more than 2 threads)
  • Does the code have any bugs I can't see?
  • Any way to write it much simpler?

Work.java

public class Work {

    //public start() and stop() methods that can be accessed
    //from two threads:
    //- JavaFX main thread (user action)
    //- other thread (after all of the task are done)
    //
    //Those methods return a boolean that indicates if
    //the chosen action was performed properly
    //(just to show an alert to user) 

    public static boolean start() {
        return processQuery(Query.START);
    }

    public static boolean stop() {
        return processQuery(Query.STOP);
    }

    //------Internals

    private static final QueryExecutor queryProcessor = new QueryExecutor(Work::unlockProcessing);
    private static final WorkState state = new WorkState();
    private static final Object lock = new Object();

    private static boolean processQuery(Query query) {
        synchronized (lock) {
            if (state.isInProperState(query)) {
                state.changeState(query);
                queryProcessor.execute(query == Query.START ? Work::startQuery : Work::stopQuery);
                return true;
            } else {
                return false;
            }
        }
    }

    private static void unlockProcessing() {
        synchronized (lock) {
            state.matureState();
        }
    }

    private static void startQuery() {
        // <SOME SYNCHRONOUS CODE>
    }

    private static void stopQuery() {
        // <SOME SYNCHRONOUS CODE>
    }

}

WorkState.java

public class WorkState {

    private State state = State.NOT_RUNNING;

    public void changeState(Query query) {
        state = query == Query.START ? State.INITIALIZING : State.FINALIZING;
    }

    public void matureState() {
        state = state == State.INITIALIZING ? State.RUNNING : State.NOT_RUNNING;
    }

    public boolean isInProperState(Query query) {
        return (state == State.RUNNING && query == Query.STOP)
                || (state == State.NOT_RUNNING && query == Query.START);
    }

    private enum State {
        INITIALIZING, RUNNING, FINALIZING, NOT_RUNNING;
    }

}

QueryExecutor.java

public class QueryExecutor extends ThreadPoolExecutor {

    private final Runnable unlockProcessingAfterExecute;

    public QueryExecutor(Runnable unlockProcessingAfterExecute) {
        super(1, 1, 0L,
                TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>());
        this.unlockProcessingAfterExecute = unlockProcessingAfterExecute;
    }

    @Override
    protected void afterExecute(Runnable r,
                                Throwable t) {
        super.afterExecute(r, t);
        unlockProcessingAfterExecute.run();
    }

}

Query enum

public enum Query {
    START, STOP
}
\$\endgroup\$
1
  • \$\begingroup\$ How is this related to JavaFX? \$\endgroup\$
    – SedJ601
    Jan 19, 2021 at 5:49

1 Answer 1

3
\$\begingroup\$

Please take a look at this library for inspiration on how to state machine could be defined: https://projects.spring.io/spring-statemachine/

  • Should I change locking system to "fair"? => Could you please clarify what you are trying to achieve
  • Will it work smoothly with 2 threads? (There's no possibility that the start()/stop() can be accessed at the same time by more than 2 threads) => Yes. You have declared the only monitor object (lock) and configured corePoolSize = 1, maximumPoolSize = 1.
  • Does the code have any bugs I can't see? => I do not understand what is the purpose of this exercise
  • Any way to write it much simpler? => I think yes. Work is a singleton. the synchronized keyword could be used directly on Work's methods. And without clarification, it looks like that the whole solution could be replaced with ThreadPoolExecutor, cause it is not clear why do we need RUNNING, FINALIZING states.
\$\endgroup\$
1
  • \$\begingroup\$ 1. Non-fair locks can potentially cause starvation. As you can see in my code, lock object can be used at the same time by 3 threads (JavaFX main, other thread and the second other thread from QueryExecutor). That could potentially lead to starvation \$\endgroup\$
    – WLTY
    Aug 7, 2020 at 12:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.