Java, Make a method that can only be called once at a time

Question

I have an application that uses a read write lock to prevent other methods from running when the so called "reconnect" method is called. Now, I want this "reconnect" method to only be called once at a time. For example, when Thread1 calls "reconnect" and while "reconnect" is executing, Thread2 calls it, it either immediately returns or waits until the call of Thread1 is finished and then returns. (Resulting in only one execution). As you can probably imagine I have an application that interacts with some API and when my session times out, it needs to reconnect but I don't want every thread creating a new connection since that would be completely unnecessary. I hope I provided enough information.

consider synchronization: http://stackoverflow.com/questions/1085709/what-does-synchronized-mean — Robb, Apr 24 '14 at 18:18
Maybe these answers help you: http://stackoverflow.com/questions/13356702/how-can-i-make-sure-a-method-is-only-called-once-by-multiple-threads — Peter Frauenknecht, Apr 24 '14 at 18:19

score 5 · Accepted Answer · answered Apr 24 '14 at 18:23

5

It's preferable to maintain a lock object and synchronize on that, such as:

public class MyClass {

    private final Object lock = new Object();

    public void reconnect() {
        synchronized(lock) {
            ....
        }
    }

    ....
}

The reason is that when you use the synchronized keyword in a method signature, you're actually synchronizing on your MyClass instance. The problem is, any other code outside your class could also do that. For instance,

public class SomeOtherClass {
    public void go(final MyClass myClass) {
        synchronized(myClass) {
            wait(Integer.MAXIMUM_VALUE);
        }
    }
}

Now if some other thread wants to call myClass.reconnect(), they can't, because SomeOtherClass has taken the lock on the myClass instance.

answered Apr 24 '14 at 18:23

Eric Stein

13,209
3
37
52

Way to dredge up one of the darkest corners of Java, lol. It's horrible practice to synchronize on the class definition of some other class, but you are very correct. – jgitter Apr 24 '14 at 18:25
1

It happens by accident all the time, and it's a decent attack vector if somebody is trying to break your code. – Eric Stein Apr 24 '14 at 18:26
Note that I already have a ReentrantReadWriteLock integrated in the class in question. Would tryLock work the same as your solution? I use the write part only for reconnecting. The rest of the methods that use the connection, use read.lock() and read.unlock(); – Martijn Apr 24 '14 at 18:26
Yes, using a ReadWrite lock would be clearly better than synchronization. – Eric Stein Apr 24 '14 at 18:27
Awesome thanks for your great help. I'll be accepting this as the answer asap. – Martijn Apr 24 '14 at 18:28
@EricStein +1 Hackers, oh my! – jgitter Apr 24 '14 at 18:43

score 4 · Answer 2 · answered Apr 24 '14 at 18:14

4

Synchronized keyword prevents multiple threads from executing the function at the same time. Then you just need to check the state of the connection in your method.

public synchronized void reconnect() {
    if (!connection.isActive()) { // this method will depend on what type of connection you have
        // your code ...
    }
}

Essentially you just need reconnect() to be a no-op if the connection is already active.

answered Apr 24 '14 at 18:14

jgitter

3,396
1
19
26

Thanks, didn't know the answer could be so simple :) – Martijn Apr 24 '14 at 18:18
It gets trickier when you have multiple functions acting upon a single resource that need to be synchronized together, but this is the simple case. Cheers! – jgitter Apr 24 '14 at 18:19
I guess I'm gonna use your connection.isActive idea together with tryLock. I think that will work the best in this situation. – Martijn Apr 24 '14 at 18:20

score 0 · Answer 3 · answered Apr 24 '14 at 20:05

I would probably not use a direct lock.

I would use just one thread to handle all the connect/reconnect actions and queue request objects to it. This is a little more complex, but:

Its more flexible because such a mechansim would allow both synchronous and asynchronous requests/replies.
It's easier to debug because the connect/reconnect only ever happens serially on one thread.
It's easy to add an extra connection to [whatever] if it's ever required or acceptable.
Timeouts, keep-alives and connection problems can be detected, and logged/rectified, even if there are no requests outstanding.
Plain hard-locks over operations that may potentially block for an extended time are just too worrying.

Maybe it's just me... :)

score -1 · Answer 4 · answered Apr 24 '14 at 18:18

You need a flag and synchronization:

private boolean mReconnectNeeded;

public synchronized void reconnectIfNeeded() {
    if (mReconnectNeeded) {
        mReconnectNeeded = false;
        // reconnect; if it fails, set the flag again
    }
}

public synchronized void setReconnectNeeded() {
    mReconnectNeeded = true;
}

Java, Make a method that can only be called once at a time

4 Answers4