Thread DEADLOCK problems! Help!

[DiscoStu]

Hello everyone,

Im writing an online computer game and Im having some deadlock
trouble on
my servers. I am using multiple threads and I am having some
deadlock trouble with my current design. This design will work for a
short time
but quickly deadlocks at random times, the smaller I make the
Thread.sleep() values the quicker it starts to deadlock so Im thinking
maybe starvation but Im not sure. Here is some pseudocode of what I am
currently doing:

In the main game class there is a Vector containing the list
of all Player objects. This is referenced in the ThreadPoolManager
class below.
Im using vectors as a throwback before I knew they moved onto
ArrayList... but
Vectors are suppose to be thread safe so I stuck with it.


------------------------------------------------------
---- This object contains all the specific information
---- about the player and all of the various services
---- needed to process the users inputs
------------------------------------------------------

public class Player
{
PlayerExecutionThread pet = null;
Socket theSocket;
ObjectInputStream ois;
ObjectOutputStream oos;

public Player() { // Assume all socket stuff above is hooked up }
public void processPlayer() { // Process any messages the user sent to
the server }
public synchronized setPlayerThread(PlayerExecutionThread exe) { pet =
exe; }
public synchronized PlayerExecutionThread getPlayerThread() { return
pet; }
}

----------------------------------------------------------------
---- There are a limited amount of PlayerExecutionThread objects
---- running inside of a thread pool, and created and dished out as
needed
---- I guess this would be considered one of several "consumer"
---- objects. The run method loops and loops and consumes pPlayer
---- objects as they are assigned to this object-thread from
---- the ThreadPoolManager.
----------------------------------------------------------------

public class PlayerExecutionThread implements Runnable
{
Player pPlayer = null;
boolean bRunPlayerThread = true;

public synchronized void setPlayer(Player pEndUser) { pPlayer =
pEndUser; }
public synchronized Player getPlayer() { return pPlayer; }

public run() {
while(bRunPlayerThread == true) {

if(getPlayer() != null) {
if(pPlayer.ois.available() > 0) {
pPlayer.processPlayer();
}

// The processing for this player is finished
pPlayer.setPlayerThread(null);
playerThreadPool.returnObject(this);
pPlayer = null;
}

// Give some sleep time to not hog the thread time
Thread.sleep(500);
}
}
}

---------------------------------------------------------------------
---- This object-thread scans through the list of online players
---- looking for Player objects that dont currently have
PlayerExecutionThread
---- objects attached to them to facilitate processing.
---------------------------------------------------------------------

public class ThreadPoolManager implements Runnable
{
boolean bRunManager = true;

public run() {
while(bRunManager == true) {

synchronized(MainOnlinePlayerList) {
for(int i=0; i<MainOnlinePlayerList.size(); etc) {
Player p = (Player)MainOnlinePlayerList.elementAt(i);

if(p.getPlayerThread() == null) {

// Get an execution thread off the pool
PlayerExecutionThread pet = null;
try { pet = (PlayerExecutionThread)playerThreadPool.borrowObjec(); }
catch(Exception exc) { exc.printStackTrace(System.out); }

// Let the player and the thread know about each other
if(pet != null) {
pet.setPlayer(p);
p.setPlayerThread(pet);
}
}
}
}

// Sleep before the while flips over
Thread.sleep(smallTime);
}
}



Any help would be very appreciated.. I've read about the
wait()/notify()/notifyAll() methods and have tried adding that
functionality but got some errors.... whats the best way to
deadlock-proof my design?

Thanks,

(e-mail address removed)

 

[Gordon Scott]

Your best bet is to start throwing in a lot of print/log statements to see
what is going on before things go haywire.

However,

From what you've posted and said about your design, having threads directlly
interacting and setting values
in another thread is a major warning flag for me. IE your ThreadManager
controlling player objects, PlayerThreads,
and Players.

You might want to try simplifying your design.
Try getting rid of your ThreadPoolManager and ThreadPool altogether. Let
the PlayerExecutionThreads get their
own Player's to process.


Since you are only creating a finite number of PlayerExecutionThreads
anyways try something as follows.

1. Maintain a queue of Player objects.. you can do this with a vector, pop
from element 0, add to the end.
2. Implement the PlayerExecutionThread run method as follows.
a. Pop the first player item off of the Queue (SYNCHRONIZE)
b. Process the player
c. Add the player item back to the end of the Queue (SYNCRHONIZE)
d. Sleep or Yield.
3. Start all your PlayerExecutionThreads

This will do exactlly what you are already doing, but remove both the thread
pool and thread pool manager and
reduce complicated thread interactions.

Once you start the PlayerExecutionThreads the simply run continously by
asking for the next player in line, and
all your management dissappears. You'll probably also want to add a flag to
the PET's to quit processing when you want to terminate. So it becomes.

a. Should I quit?
b. Get next player
c. Process player
d. Add player to end of queue
e. Sleep


HTH.

Gordon.

 

[Thomas G. Marshall]

Some ideas.

I'm not sure, but this confuses me at first glance. You're synchronizing on
MainOnlinePlayerList, in only one place, and it is in a single non-recurring
thread, which doesn't provide protection. I don't know what you're trying
to protect.

PlayerExecutionThread.set,getPlayer() synchronizes on the
PlayerExecutionThread /instance/, not across all instances.

Finally, is it possible that the socket is blocking somewhere you weren't
expecting?

Thomas

PS. Where you pasting code copied from Eclipse? Apps like that woefully
**** up the indentation, even if they are only spaces: Try pasting it to
notepad first and then recopy and paste it into your message here.

 

[Thomas G. Marshall]

I forgot something:

When in doubt, use println() statements to see where the block is occuring.

It's fairly easy to do, and can be done with little thought as to where they
are being placed: just cut and paste here and there.

As it turns out, System.out.println() is synchronized such that it will not
be interrupted mid call by another system.out.println(), which makes things
easier.

Note, though, that the mere presence of the println's will change the
behavior of your app considerably, and is known to move or even completely
remove race conditions, which is a true pain in the ass, so don't put too
many in at first.

Thomas

 

[Ian]

Run your game from the command shell and hit CTRL-BREAK to dump the active
threads; you should see your deadlocks.

 

[Thomas G. Marshall]

Run your game from the command shell and hit CTRL-BREAK to dump the
active threads; you should see your deadlocks.

....[kershnipenzie]...

GREAT.

I've /always/ wanted to become versed in this ctrl-break thing. Is there a
website that you suggest that shows specifically how to use this information
for fault-isolating race conditions?

I've previously used println's or VisualCafe's threading (+thread stack)
information, but it was horrifyingly difficult.

 

[Kari S]

Any help would be very appreciated.. I've read about the
wait()/notify()/notifyAll() methods and have tried adding that
functionality but got some errors.... whats the best way to
deadlock-proof my design?

Thanks,

(e-mail address removed)

Hi

I would suggest that you read again about wait() / notify() / notifyAll()
methdods. Thread.sleep() never release the lock the tread has. I believe
that using these methods is not only the best way to deadlock-proof your
design, it's the Only way to avoid deadlocks.

So, without studying your code any deeper I think you should really get back
to the drawing board and study thread synchronization before you continue
patching your code.

KS

 

[soft-eng]

Hello everyone,

Im writing an online computer game and Im having some deadlock
trouble on
my servers. I am using multiple threads and I am having some
deadlock trouble with my current design. This design will work for a
short time
but quickly deadlocks at random times, the smaller I make the
Thread.sleep() values the quicker it starts to deadlock so Im thinking
maybe starvation but Im not sure. Here is some pseudocode of what I am
currently doing:

In the main game class there is a Vector containing the list
of all Player objects. This is referenced in the ThreadPoolManager
class below.
Im using vectors as a throwback before I knew they moved onto
ArrayList... but
Vectors are suppose to be thread safe so I stuck with it.


------------------------------------------------------
---- This object contains all the specific information
---- about the player and all of the various services
---- needed to process the users inputs
------------------------------------------------------

public class Player
{
PlayerExecutionThread pet = null;
Socket theSocket;
ObjectInputStream ois;
ObjectOutputStream oos;

public Player() { // Assume all socket stuff above is hooked up }
public void processPlayer() { // Process any messages the user sent to
the server }
public synchronized setPlayerThread(PlayerExecutionThread exe) { pet =
exe; }
public synchronized PlayerExecutionThread getPlayerThread() { return
pet; }
}

----------------------------------------------------------------
---- There are a limited amount of PlayerExecutionThread objects
---- running inside of a thread pool, and created and dished out as
needed
---- I guess this would be considered one of several "consumer"
---- objects. The run method loops and loops and consumes pPlayer
---- objects as they are assigned to this object-thread from
---- the ThreadPoolManager.
----------------------------------------------------------------

public class PlayerExecutionThread implements Runnable
{
Player pPlayer = null;
boolean bRunPlayerThread = true;

public synchronized void setPlayer(Player pEndUser) { pPlayer =
pEndUser; }
public synchronized Player getPlayer() { return pPlayer; }

public run() {
while(bRunPlayerThread == true) {

if(getPlayer() != null) {
if(pPlayer.ois.available() > 0) {
pPlayer.processPlayer();
}

// The processing for this player is finished
pPlayer.setPlayerThread(null);
playerThreadPool.returnObject(this);
pPlayer = null;
}

// Give some sleep time to not hog the thread time
Thread.sleep(500);
}
}
}

---------------------------------------------------------------------
---- This object-thread scans through the list of online players
---- looking for Player objects that dont currently have
PlayerExecutionThread
---- objects attached to them to facilitate processing.
---------------------------------------------------------------------

public class ThreadPoolManager implements Runnable
{
boolean bRunManager = true;

public run() {
while(bRunManager == true) {

synchronized(MainOnlinePlayerList) {
for(int i=0; i<MainOnlinePlayerList.size(); etc) {
Player p = (Player)MainOnlinePlayerList.elementAt(i);

if(p.getPlayerThread() == null) {

// Get an execution thread off the pool
PlayerExecutionThread pet = null;
try { pet = (PlayerExecutionThread)playerThreadPool.borrowObjec(); }
catch(Exception exc) { exc.printStackTrace(System.out); }

// Let the player and the thread know about each other
if(pet != null) {
pet.setPlayer(p);
p.setPlayerThread(pet);
}
}
}
}

// Sleep before the while flips over
Thread.sleep(smallTime);
}
}



Any help would be very appreciated.. I've read about the
wait()/notify()/notifyAll() methods and have tried adding that
functionality but got some errors.... whats the best way to
deadlock-proof my design?

Thanks,

(e-mail address removed)

Your 2 "synchronized"s in Player and 2 in PlayerExecutionThread are
essentially doing nothing (you can remove them). As the only other
"synchronized" is on MainOnlinePlayerList, if you indeed
have a deadlock, it will be coming from whoever else
is synchronizing on MainOnlinePlayList. That is not
included in your code fragment above.

I would recommend writing some smaller programs first
using synchronized, wait, notify etc, until you are
clear on the concepts. This one seems like too
complex for a first try. For instance: See if you can
use only "synchronize" to get one very simple thread to
print a line in an infinite loop, but make this thread
wait for another thread that reads a line from the user
in an infinite loop. (First thread may output the line
several times before waiting, but sooner or
later it must wait for user input.)

After gaining some experience with little programs
like this, you should be able to design something like
this more easily.

 

[Mark Meyer]

In our last episode, "Thomas G. Marshall"

...
Note, though, that the mere presence of the println's will change the
behavior of your app considerably, and is known to move or even completely
remove race conditions, which is a true pain in the ass, so don't put too
many in at first.

A bug that changes its behavior or disappears when one tries to
isolate it is what I like to call a "heisenbug".


Mark Meyer (e-mail address removed)
Raytheon Voice (972)344-0830 Fax (972)344-6840

 

[Thomas G. Marshall]

In our last episode, "Thomas G. Marshall"


A bug that changes its behavior or disappears when one tries to
isolate it is what I like to call a "heisenbug".

LOL

I might use that.

 

[David Zimmerman]

LOL

I might use that.

I think of them as cockroaches, as soon as you turn on the light, they
hide under the fridge.