deadlock??

[tcl]

The following is not my code, but I have to maintain it.
The two classes have been simplified to get to the point,
try-catch blocks, among other things, are omitted.
MyStuff has a main thread run(). And clients call
addNotification() at times.
Calling the addNotification() method in a myStuff object
sometimes hangs indefinitely.
Questions:
- should the calls mySem.release()
and mySem.acquire() be enclosed inside the synchronized blocks?
- can anyone tell me what could cause the deadlock or hanging?
Thanks a million,
ted

class semaphore
{
private long myCount = 0;
public semaphore() {myCount = 0;}
public void acquire()
{
synchronized(this)
{
while(myCount == 0){ wait(); }
myCount--;
}
}
public synchronized void release()
{
myCount++;
notify();
}
}

class myStuff
{
private LinkList myList=null;
private semaphore mySem = null;
public myStuff()
{
myList = new LinkList();
mySem = new semaphore()
}
public void addNotification(xyz a)
{
sychronized(myList)
{
myList.addList(a);
}
mySem.release();
}
public void run()
{
while(myThreadRun)
{
mySem.acquire();
xyz ref=null;
synchronized(myList)
{
ref = myList.removeFirst();
}
ref.foo();
}
}

 

[hiwa]

The following is not my code, but I have to maintain it.
The two classes have been simplified to get to the point,
try-catch blocks, among other things, are omitted.
MyStuff has a main thread run(). And clients call
addNotification() at times.
Calling the addNotification() method in a myStuff object
sometimes hangs indefinitely.
Questions:
- should the calls mySem.release()
and mySem.acquire() be enclosed inside the synchronized blocks?
- can anyone tell me what could cause the deadlock or hanging?
Thanks a million,
ted

class semaphore
{
private long myCount = 0;
public semaphore() {myCount = 0;}
public void acquire()
{
synchronized(this)
{
while(myCount == 0){ wait(); }
myCount--;
}
}
public synchronized void release()
{
myCount++;
notify();
}
}

class myStuff
{
private LinkList myList=null;
private semaphore mySem = null;
public myStuff()
{
myList = new LinkList();
mySem = new semaphore()
}
public void addNotification(xyz a)
{
sychronized(myList)
{
myList.addList(a);
}
mySem.release();
}
public void run()
{
while(myThreadRun)
{
mySem.acquire();
xyz ref=null;
synchronized(myList)
{
ref = myList.removeFirst();
}
ref.foo();
}
}

Post a small demo code that is generally compilable, runnable and could
reproduce your problem. See:
http://homepage1.nifty.com/algafield/sscce.html and
http://www.yoda.arachsys.com/java/newsgroups.html

 

[Thomas Hawtin]

class semaphore

It would be a good deal less confusing if you used initial caps for
class names.

There already is a Semaphore class in the Java library. Why not use
that? If you are using a really old version of Java, then there is the
JSR166 backport.

{
private long myCount = 0;
public semaphore() {myCount = 0;}

Why assign myCount twice. In fact, why assign it at all. Without the
assignment, you could get away with unsafe publishing.

public void acquire()
{
synchronized(this)
{
while(myCount == 0){ wait(); }
myCount--;
}
}
public synchronized void release()
{
myCount++;
notify();
}
}

class myStuff
{
private LinkList myList=null;

The Java library already has a linked list class.

private semaphore mySem = null;
public myStuff()
{
myList = new LinkList();
mySem = new semaphore()

Again why assign twice? Marking the fields final would be a good idea.

}
public void addNotification(xyz a)
{
sychronized(myList)
{
myList.addList(a);
}
mySem.release();
}
public void run()
{
while(myThreadRun)
{
mySem.acquire();
xyz ref=null;

No need to assign ref here. In fact, it could be made final.

synchronized(myList)
{
ref = myList.removeFirst();
}
ref.foo();
}
}

At no point in this code to acquire either the semaphore or LinkList
intrinsic lock at the same time, so that isn't deadlocking. Does jstack,
ctrl-\ (ctrl-break on Windows) or your debugger give any clues?

There isn't any need to move the acquire into the synchronized block.

I suggest using the much better BlockingQueue implementations supplied
in the Java library (and JSR166 backport).

Tom Hawtin