Skip to main content
GameDev.net gamedev.net
🔒 Locked

[java] Multithreaded Problem [Solved]

Started by Sparhawk42 Sep 16, 2005 at 5:13 AM 2 replies 500+ views
Original Post
Sparhawk42
Sparhawk42
Hello people, I've a small problem with my multithreaded server... I've one thread who accepts requests and pushes them into a vector:

private Vector jobList_;
private java.util.concurrent.Semaphore jobListSema_;
private java.util.concurrent.Semaphore jobListEmptySema_;

	public void pushJob( JobEntry job )
	{
		try
		{		
			jobListSema_.acquire();
			jobList_.add( job );
			jobListSema_.release();
			jobListEmptySema_.tryAcquire();
			jobListEmptySema_.release();
			
		} catch( InterruptedException e )
		{
		}
	}
With the following initialization in my constructor:

jobList_ = new Vector();
		jobListSema_ = new java.util.concurrent.Semaphore( 1 );
		jobListEmptySema_ = new java.util.concurrent.Semaphore( 1 );

		jobListEmptySema_.tryAcquire();
And i've an other thread, who removes those jobs:

public JobEntry popJob()
	{
		JobEntry res;
		try
		{
			if( jobList_.isEmpty() )
				jobListEmptySema_.acquire();
			jobListSema_.acquire();

			res = (JobEntry) jobList_.remove( 0 );
			jobListSema_.release();

		} catch( InterruptedException e )
		{
			res = null;
		}
		
		return res;
	}
And now after some time (and about 500 jobs) I get:

Exception in thread "Thread-0" java.lang.ArrayIndexOutOfBoundsException: Array i
ndex out of range: 0
        at java.util.Vector.remove(Vector.java:793)
        at LoungeServer.popJob(LoungeServer.java:173)
        at LoungeServer.run(LoungeServer.java:152)
Which is exactly the remove in popJob() But I don't understand how this can happen, as I thought the jobListEmptySema_ garantuees my jobList_ is not empty, when I acquire the Sempahore in popJob... Thanks for your help. [Edited by - Sparhawk42 on September 16, 2005 8:00:07 AM]
Sparhawk42
Sparhawk42
So I've modified my code a little, as I thought the check for isEmpty of the list should also be in the critical section, but it still doesn't work:

	public JobEntry popJob()	{		JobEntry res;		try		{			jobListSema_.acquire();			if( jobList_.isEmpty() )			{				jobListSema_.release();				jobListEmptySema_.acquire();				jobListSema_.acquire();			}					res = (JobEntry) jobList_.remove( 0 );			jobListSema_.release();		} catch( InterruptedException e )		{			System.err.println( e.toString() );			res = null;		}				return res;	}		public void pushJob( JobEntry job )	{		try		{					jobListSema_.acquire();			jobList_.add( job );			jobListEmptySema_.tryAcquire();			if( !jobList_.isEmpty() )				jobListEmptySema_.release();				jobListSema_.release();					} catch( InterruptedException e )		{			System.err.println( e.toString() );		}	}
FlowingOoze
FlowingOoze
I don't know the sematics of those semaphores, but why don't you just use the monitor of the Vector object with synchronized statement and Object.wait()? The code would be cleaner and less error prone.
Sparhawk42
Sparhawk42
Thanks a lot now it works!

I jst didn't know that java had such a lowlevel multithread support, as I've mostly programmed c/c++...

So my code looks now like this:
public JobEntry popJob()	{		JobEntry res;		synchronized( jobList_ ) {						while( jobList_.isEmpty() )			{				try				{					jobList_.wait();				}				catch( InterruptedException e )				{				}			}			res = (JobEntry) jobList_.remove( 0 );					}				return res;	}		public void pushJob( JobEntry job )	{		synchronized( jobList_ ) {			jobList_.add( job );			jobList_.notify();		}	}


Also I don't like the while-loop here, put that's just my personal feeling.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.