Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InterProcessMutex from multiple threads #31

Closed
gmalouf opened this issue Feb 23, 2012 · 12 comments
Closed

InterProcessMutex from multiple threads #31

gmalouf opened this issue Feb 23, 2012 · 12 comments

Comments

@gmalouf
Copy link

gmalouf commented Feb 23, 2012

I started testing with the InterProcessMutex today and was surprised to find out that it threw an IllegalMonitorException if another thread in the same jvm already had the lock. What I expected was behaviour within the same jvm similar to Java's standard ReentrantLock.

What are the best practices for the case where one would like the lock to be shared both within and across jvms? What I can do short term is to try/catch the illegal monitor exception everywhere I need to call the lock but that seems cludgy and is less than ideal. It seems like to get the behaviour I would really want the acquire method would need the ability to block.

@Randgalt
Copy link
Contributor

It's not correct that the JVM ReentrantLock allows multi-threaded usage. It wouldn't be very useful if it did. Maybe I'm misunderstanding your use case. Here's an example I wrote with ReentrantLock.

    public void     test() throws Exception
    {
        final CountDownLatch      latch = new CountDownLatch(2);
        final ReentrantLock       lock = new ReentrantLock();
        Thread                    t1 = new Thread
        (
            new Runnable()
            {
                @Override
                public void run()
                {
                    lock.lock();
                    latch.countDown();
                }
            }
        );
        Thread                    t2 = new Thread
        (
            new Runnable()
            {
                @Override
                public void run()
                {
                    lock.lock();
                    latch.countDown();
                }
            }
        );

        t1.start();
        t2.start();
        if ( latch.await(10, TimeUnit.SECONDS) )
        {
            System.out.println("yep");
        }
        else
        {
            System.out.println("nope");
        }
    }

@gmalouf
Copy link
Author

gmalouf commented Feb 23, 2012

Thanks for replying. The point I wanted to get across was that I felt that the acquire method should block within the same jvm like it does across jvms. If inter process mutex is called from 2 different threads at the same time in the same jvm right now, the first one will get access and the second call will result in an illegal monitor exception (rather than blocking).

I work around this with a simple java lock in my trait that wraps the mutex, just thought it would be nice if this was kept within the library.

@Randgalt
Copy link
Contributor

OK - I understand now. I consider this a bug and I'll work on a fix. As a workaround, you can allocate a new InterProcessMutex in each thread.

@gmalouf
Copy link
Author

gmalouf commented Feb 23, 2012

I wonder if it is not better to leverage Google Collections' Function class and put the acquire and release methods together.

Adding synchronized to my lock method works fine for now: (scala)

trait Lockable extends ZooKeeperHarness {

val nodePathToLockUpon: String

private[cluster] lazy val lock: InterProcessLock = {
if (!exists(nodePathToLockUpon)) create(nodePathToLockUpon, PERSISTENT)
new InterProcessMutex(zkClient, nodePathToLockUpon)
}

def lock(functionToExecuteWhileLocked: => Unit) {
//Only one thread per Lockable instance can use the InterProcessMutex (throws IllegalMonitorException otherwise)
synchronized {
lock.acquire()
try {
functionToExecuteWhileLocked
} finally {
lock.release()
}
}
}
}

The InterProcessMutex per thread solution would work as well; these locks are used infrequently so I am not really concerned about the contention.

@Randgalt
Copy link
Contributor

I've pushed a fix - I'll build new binaries when I can. Thanks for finding this.

@r7raul1984
Copy link

@Randgalt Could you show me the push url? Thanks.

@Randgalt
Copy link
Contributor

Sorry @r7raul1984 - I don't know what you mean. Also, Curator is now at Apache. See https://curator.apache.org

@r7raul1984
Copy link

@Randgalt About InterProcessMutex ,I wanted to get across was that I felt that the acquire method should block within the same jvm like it does across jvms. How to fix it? Which version of Curator support this ?

@Randgalt
Copy link
Contributor

acquire does block in the same JVM. It acts just like the JDK lock.

@r7raul1984
Copy link

@Randgalt OK. Thank you! I should create new CuratorFramework client for every InterProcessMutex. Many InterProcessMutex instance can't share one CuratorFramework client. Is that correct?

@Randgalt
Copy link
Contributor

No - 1 CuratorFramework instance is enough for your entire application.

@r7raul1984
Copy link

@Randgalt Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants