Search code examples
javamultithreadingsynchronizedjava.util.concurrent

Synchronize 2 methods


public class ActionForm {
       private Account temporaryAccount = null;
       private Document document;

       /**
       * Save document from another thread that do not have a SecurityContext
       */
       public void saveByAccount(Account account) { 
           this.temporaryAccount = account;
           save();
           this.temporaryAccount = null;
       }

       /**
       * Save document to DB. 
       * I can not change the signature of this method.
       */
       public synchronized void save() {

          //get an account from shared variable or from SecurityContext
           Account account = null;
           Account temporaryAccount = this.temporaryAccount;
           if (temporaryAccount == null) {
               account = SecurityContextWrapper.getAccount();
           } else {
               account = temporaryAccount;
           }

        //save in DB
        saveDocumentInDB(account, document);
    }
}

Thread type1: User can click button "save", in that case method save() will call directly. I get account from SecurityContext.

Thread type2: User starts a background process. I save his/her account and then start new thread:

final Account account = SecurityContextWrapper.getAccount();
new Thread(new Runnable() {
    public void run() {
        ...//do smth
        saveByAccount(account);
    }
}).start();

The problem: The variable this.temporaryAccount can be changed - between calling saveByAccount() and save(). Do you know the proper way to synchronize these methods?


Solution

  • The best way to solve this problem would be to send the account to each method as a parameter. Encapsulation is always a nice feature and you should strive for it, whenever possible. This way, when you get the need to paralelize, you won't face this kind of trouble.

    Given your comment that you cannot change the method signature, I would suggest you use a semaphore before starting to use the shared variable.

    You could create, at the class level the semaphore using the following code:

    private final Semaphore available = new Semaphore(1, true);
    

    Each method, before trying to change or use the shared variable would have to call available.acquire();. This would block if the semaphore was under use (for you have a single permit, as defined in the constructor call), but if it was free, it would then decrease the number of permits by one and continue.

    After finishing its processing that depended on the shared variable, each method should call available.release();. Then, one of the other methods waiting to be served would acquire the semaphore and continue.

    Nevertheless, I would strongly suggest that you take the time and refactor your code. Global and class variables are "code smells" and can induce errors in the future. The time spent with this refactor would pay off with interest in the future. This type of discussion is available in excelent books such as "Code Complete" and "Clean Code". They are must reads and offer us, programmers, lots of insight on code quality.

    I hope this helps.