Multithreaded synchronized deadlock in Java

I have an application that requires real time rendering. I have two methods accessed from separate threads that access a member variable in a class. However when I try running the program, eventually I get into a state where both methods have been called (i.e. synchronized has been called twice on the same element) and the second thread is blocked waiting for the first thread to release the lock on the object. Below is sample code:

public class Class {
private final Set<Object> objects;

...

public void method1() {
    synchronized(objects) {
        // do something
    }
}

public void method2() {
    synchronized(objects) {
        // do something else
    }
}
}

Is this incorrect? What is the correct way to perform these operations without causing deadlock? Thanks

EDIT: Here is the stack trace from JConsole

Name: Thread-6432
State: BLOCKED on java.util.HashSet@25a6cc45 owned by: AWT-EventQueue-0
Total blocked: 1  Total waited: 0

Stack trace: 
com.sonogenics.renderer.renderElements(Elements.java:81)
com.sonogenics.renderer.CameraHandler$Setup.run(CameraHandler.java:106)
java.lang.Thread.run(Thread.java:619)


Name: AWT-EventQueue-0
State: WAITING on java.util.concurrent.locks.ReentrantLock$NonfairSync@656c50af owned by: pool-1-thread-1
Total blocked: 11,051  Total waited: 11,232

Stack trace: 
sun.misc.Unsafe.park(Native Method)
java.util.concurrent.locks.LockSupport.park(LockSupport.java:158)
java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:811)
java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireQueued(AbstractQueuedSynchronizer.java:842)
java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(AbstractQueuedSynchronizer.java:1178)
java.util.concurrent.locks.ReentrantLock$NonfairSync.lock(ReentrantLock.java:186)
java.util.concurrent.locks.ReentrantLock.lock(ReentrantLock.java:262)
sun.awt.SunToolkit.awtLock(SunToolkit.java:236)
sun.java2d.pipe.RenderQueue.lock(RenderQueue.java:94)
sun.java2d.pipe.BufferedRenderPipe$AAParallelogramPipe.fillParallelogram(BufferedRenderPipe.java:443)
sun.java2d.pipe.PixelToParallelogramConverter.drawGeneralLine(PixelToParallelogramConverter.java:264)
sun.java2d.pipe.PixelToParallelogramConverter.drawLine(PixelToParallelogramConverter.java:62)
sun.java2d.pipe.ValidatePipe.drawLine(ValidatePipe.java:44)
sun.java2d.SunGraphics2D.drawLine(SunGraphics2D.java:2098)
com.sonogenicsArrow.preview(Arrow.java:97)
com.sonogenics.Elements.previewElements(Elements.java:116)
   - locked java.util.HashSet@25a6cc45
com.sonogenics.PreviewOverlay.render(PreviewOverlay.java:49)
com.sonogenics.VideoPanel.renderJava2DOverlays(VideoPanel.java:89)
   - locked java.util.ArrayList@22542822
com.sonogenics.VideoPanel.paintComponent(VideoPanel.java:64)
javax.swing.JComponent.paint(JComponent.java:1029)
com.sonogenics.VideoPanel.paint(VideoPanel.java:53)
javax.swing.JComponent.paintChildren(JComponent.java:862)
   - locked java.awt.Component$AWTTreeLock@4316e1c9
javax.swing.JComponent.paint(JComponent.java:1038)
javax.swing.JComponent.paintChildren(JComponent.java:862)
   - locked java.awt.Component$AWTTreeLock@4316e1c9
javax.swing.JComponent.paint(JComponent.java:1038)
javax.swing.JLayeredPane.paint(JLayeredPane.java:567)
javax.swing.JComponent.paintChildren(JComponent.java:862)
   - locked java.awt.Component$AWTTreeLock@4316e1c9
javax.swing.JComponent.paintToOffscreen(JComponent.java:5131)
javax.swing.RepaintManager$PaintManager.paintDoubleBuffered(RepaintManager.java:1479)
javax.swing.RepaintManager$PaintManager.paint(RepaintManager.java:1410)
javax.swing.RepaintManager.paint(RepaintManager.java:1224)
javax.swing.JComponent.paint(JComponent.java:1015)
java.awt.GraphicsCallback$PaintCallback.run(GraphicsCallback.java:21)
sun.awt.SunGraphicsCallback.runOneComponent(SunGraphicsCallback.java:60)
sun.awt.SunGraphicsCallback.runComponents(SunGraphicsCallback.java:97)
java.awt.Container.paint(Container.java:1780)
java.awt.Window.paint(Window.java:3375)
com.sonogenics.Demonstrator.paint(nDemonstrator.java:234)
javax.swing.RepaintManager.paintDirtyRegions(开发者_JAVA百科RepaintManager.java:796)
javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:713)
com.sonogenics.RepaintManager.paintDirtyRegions(RepaintManager.java:64)
javax.swing.RepaintManager.seqPaintDirtyRegions(RepaintManager.java:693)
javax.swing.SystemEventQueueUtilities$ComponentWorkRequest.run(SystemEventQueueUtilities.java:125)
java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:209)
java.awt.EventQueue.dispatchEvent(EventQueue.java:597)
java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:269)
java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:184)
java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:174)
java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:169)
java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:161)
java.awt.EventDispatchThread.run(EventDispatchThread.java:122)

EDIT 2: I should also note that the two methods do nothing but iterate through the set.

The above code as such by itself will never deadlock because there is only one synchronization object i.e. "objects".

If thread2 is blocked then its simply because thread1 has not finished method1/method2.

When its blocked, just print the thread dump and analyze (or post here ?).

If there is some other synchronization factor within method1 or method2, then you need to mention that.

EDIT : In your thread-dump, I can see that the thread is actually blocked on awtLock. This looks like a threading issue of Swing.

Both methods use the monitor of the same Set instance. So as a sideeffect - if one thread works in method1, both blocks in method1 and method2 are locked for any other thread.

If thread1 enters method1, he enters the sets monitor. And thread1 is allowed to enter the block in method2 (method2 may be called from inside the block in method1). But all other threads will have to wait until thread1 (finally) exits the monitor.

Synchronizing several blocks on one object may be necessary, like if both blocks do modifications on the set and you don't want to have code from both methods be executed in parallel.

I hope both the threads are working on the same instance of class Class.

Unless the "do something" sections use other locks, your code shouldn't cause any deadlock. The second thread might have to wait for the first one to complete the synchronized section of the code, but this is not a deadlock : once the first thread has finished, the second thread will be woken up and complete its work.

If you want to avoid contention, you should have a look at concurrent collections in the java.util.concurrent package.

From your description it is not clear whether you have a real deadlock or just a long held lock by the first thread. If you do a time consuming operation in these methods, it may just be a side effect that other threads need to wait excessively.

So you may need to limit the lock intervals to avoid contention. Without knowing what is actually going on inside the methods, it is hard to give more concrete advice. One possibility may be to use a concurrent Set implementation such as ConcurrentSkipListSet or CopyOnWriteArraySet.

If it is not just a long held lock though, check whether objects is published to external parties. Publishing and locking on it somewhere else may indeed cause a deadlock.

From the code sample, there is no deadlock involved at all:
It is perfectly normal for the second method trying to get the lock on the object to be blocked while the first method still has it.

For deadlock to occur, 4 conditions must apply:

• Mutual exclusion [as you have with the object - only one can hold it at a time]
• hold and wait [you can request new locks while holding on to others]
• no preemption [you cannot forcibly take a lock
• and finally, Circular Wait [ resource 1 is waiting for resource 2 to release a lock, resource 2 is waiting on resource n to release a lock, and resource n is waiting on resource 1 to release its lock]

Since the first 3 are potentially present in your code sample, I would look at is to see whether there are further calls within each method that are using resources that the other has a lock on; or whether there are time consuming operations going on that are appearing to give a deadlock, when in fact they are not.