1

How can I lock a method until an other method is invoked?

public class TestLock{
    public void methodIsLockedAfterFirstCall(){
        doSomething();
        //now lockThisMethod (when invoked, the thread shall sleep)
    }
     public void methodToDoSomethingAfterTheFirstMethod(){
         doSomeOtherThing()
         //unlock the first Method?
     }
}

Is there something already in Java for this or should I do it some way around ( e.g. Boolean or AtomicLong)?

5
  • 1
    Can you describe in words what lockThisMethod should do? If someone tries to run the first method while it's locked, should it sleep until someone else runs the second one? (or something else?) Will they always be paired 1:1 first and second? Can the second run more often? Commented Jun 28, 2021 at 11:15
  • You can make methods synchronized (public synchronized ...) in which case only a single thread at the same time can execute that method. You can also synchronize on an object (synchronized(myObject) { ... }). This guarantees that the enclosed code block can only be executed single threaded. On top of this you can implement semaphores and locks. Commented Jun 28, 2021 at 11:16
  • @dratenik it shall sleep then when it is invoke but "locked" Commented Jun 28, 2021 at 11:19
  • 1
    Sounds like you are trying to reinvent Condition variables. Commented Jun 28, 2021 at 11:27
  • @dratenik a Semaphore cannot be used for this. There is no such thing as 'a semaphore with only 1 permit'. The param you pass the constructor only defines the initial permit count. .release() is 'permit++;` regardless of what you passed there. You can't 'conditionally add 1 permit if the permit count is 0', the semaphore API doesn't offer this option. Commented Jun 28, 2021 at 11:52

2 Answers 2

1

If you want to build this up from base principles you'd do something like:

private final AtomicBoolean lock = new AtomicBoolean();

public void methodIsLockedAfterFirstCall() {
    doSomething();
    synchronized (lock) {
        while (lock.getAndSet(true)) try {
            lock.wait();
        } catch (InterruptedException e) {
            return; // SEE NOTE 1
        }
    }
}

public void methodToDoSomethingAfterTheFirstMethod() {
    doSomeOtherThing();
    synchronized (lock) {
        lock.set(false):
        lock.notifyAll();
    }
}

This code:

  • Uses a private lock. Locking om something public is only acceptable if you document this behaviour and maintain this behaviour for future versions (or mark your new version as utterly incompatible with the old). As a rule, public locks are an error. synchronizing on this, therefore, is usually wrong. This code locks on a private variable.

  • This code does not run afoul of JMM issues by using AtomicBoolean.

  • NOTE 1: InterruptedException only ever occurs if you (or other code running on the JVM) explicitly calls .interrupt() on the thread (it does not occur if e.g. the user hits CTRL+C, or killall YourProcess, or 'end task' in the task manager, or any other way that doesn't involve code running in that VM that calls .interrupt(). What to do? Well, do not just e.printStackTrace(), the usual mainstay of java programmers who no idea what they are doing. What did you want to happen when you write thread.interrupt(), somewhere else in the codebase? Do that. If the notion of 'stop waiting for that second call now' is a sensible idea, then document the behaviour in this method. Here I've chosen to just return (stop waiting), but keep the lock in locked state.

  • Does not use notify/wait as a mechanism to communicate data; only as a mechanism to communicate when to wait and when to stop waiting. This is generally a good idea, it can be very hard to debug relevant state when that state is captured by 'were you notified or not', and makes it impossible to use the wait(timeout) variant. That's why there is a while loop. Being woken up just results in trying to getAndSet again, which can reuslt in waiting some more. That's a good thing.

Or, use something from j.u.concurrent. Some ideas:

  • A Lock which the first method locks and the second method unlocks.
  • A Semaphore doesn't sound right, as .release() will add 1 to the count, always, so if you call the second method whilst the 'lock status' is UNLOCKED, you'd erroneously be adding a permit. You can't do if (semaphore.availablePermits() < 1) semaphore.release(); as that'd have a race condition unless you do this in a synchronized block which kinda defeats the purpose.
Sign up to request clarification or add additional context in comments.

5 Comments

I get a headache looking at this code. I would not conflate the lock and the AtomicBoolean. I would not even use an AtomicBoolean in this case but just a non volatile boolean flag.
@pveentjer I'm not sure why you felt the need to insult this answer. Perhaps add some insights as to why. Otherwise, you're just espousing your personal tastes. There's not much point arguing about taste, which is why I suggest you don't write such comments. You know.. leaves a bad taste :P
My main problem is that while you are inside a lock, you are executing a synchronizing operation (getAndSet) which isn't needed. Normally when I see a synchronizing operation, I know I need to be careful because it can be called concurrent. But in this case it isn't possible due to the outer lock. So when I read this code, I start to doubt my own sanity.
The costs are effectively irrelevant, and the benefits are real: If you want to check if the lock is currently locked, you can just call .get() on that thing. It's a little dubious to want this info without acquiring locks, admittedly, as out of date by the time you do anything with it if you don't. It's also a single field instead of two. Perhaps just adjust, er, your sanity? A more esoteric / stylistic advantage is that the AtomicBoolean type suggests (correctly) that it's a field involved in concurrency-sensitive operations.
I didn't mentioned cost. I explained that the code is difficult to understand because you are using an atomic getAndSet without without being a synchronization operation and you are using a lock free atomic as a lock based data-structure. So you are flipping the nature of the data structure inside out.
0

There is a lot under the java.util.concurrent.locks and java.util.concurrent packages.

Maybe CountDownLatch is the easier one to use:

private final CountDownLatch latch = new CountDownLatch(1);

public class TestLock{
    public void methodIsLockedAfterFirstCall() throws InterruptedException {
        doSomething();
        //now lockThisMethod (when invoked, the thread shall sleep)
        latch.await()
    }
    public void methodToDoSomethingAfterTheFirstMethod(){
        doSomeOtherThing()
        //unlock the first Method?
        latch.countDown();
    }
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.