river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Firmstone <j...@zeus.net.au>
Subject Re: com.sun.jini.thread lock contention
Date Wed, 09 Jun 2010 06:39:44 GMT
Hey Thanks Chris,  Awesome to see some contribution.

I committed something very similar last night, it could use some 
refactoring too, the only difference from what you have here is the 
TaskThread creates the thread in its constructor, but it doesn't start 
the thread in the constructor.  I added an additional method 
TaskTread.start() to start the thread.  It avoids the null reference and 
thread could be final, can't remember if I made the Thread reference 
final or not.

Oddly enough though , Hudson failed shortly after, so I'll need to 
investigate why, here's the Hudson web site:

http://hudson.zones.apache.org/hudson/view/River/job/River-trunk/

Cheers,

Peter.

Christopher Dolan wrote:
> OK, sorry for the repeated messages. The list apparently doesn't allow
> attachments...
>
> --- TaskManager.java	2010-03-02 11:49:30.629703600 -0600
> +++ TaskManager2.java	2010-06-08 11:11:54.309412900 -0500
> @@ -190,10 +190,13 @@
>  	logAdd(t);  //DAS 6/06 add
>  	boolean poke = true;
>  	while (threads.size() < maxThreads && needThread()) {
> -	    Thread th;
> +	    TaskThread th;
>  	    try {
>  		th = new TaskThread();
> -		th.start();
> +		Thread thread = new Thread(new TaskThread());
> +		thread.setName("task");
> +		thread.setDaemon(true);
> +		thread.start();
>  	    } catch (Throwable tt) {
>  		try {
>  		    logger.log(threads.isEmpty() ?
> @@ -290,8 +293,8 @@
>  		    for (int j = threads.size(); --j >= 0; ) {
>  			TaskThread thread = (TaskThread)threads.get(j);
>  			if (thread.task == t) {
> -			    if (thread != Thread.currentThread())
> -				thread.interrupt();
> +			    if (thread.myThread !=
> Thread.currentThread())
> +				thread.myThread.interrupt();
>  			    break;
>  			}
>  		    }
> @@ -327,15 +330,11 @@
>  	return maxThreads;
>      }
>  
> -    private class TaskThread extends Thread {
> +    private class TaskThread implements Runnable {
>  
>  	/** The task being run, if any */
>  	public Task task = null;
> -
> -	public TaskThread() {
> -	    super("task");
> -	    setDaemon(true);
> -	}
> +	public Thread myThread = null;
>  
>  	/**
>  	 * Find the next task that can be run, and mark it taken by
> @@ -363,8 +362,9 @@
>  	}
>  
>  	public void run() {
> +	    myThread = Thread.currentThread();
>  	    while (true) {
>  		synchronized (TaskManager.this) {
>  		    if (terminated)
>  			return;
>  		    if (task != null) {
> @@ -376,11 +376,11 @@
>  			    }
>  			}
>  			task = null;
> -			interrupted(); // clear interrupt bit
> +			myThread.interrupted(); // clear interrupt bit
>  		    }
>  		    if (!takeTask()) {
>  			try {
>  			    TaskManager.this.wait(timeout);
>  			} catch (InterruptedException e) {
>  			}
>  			if (terminated || !takeTask()) {
>
>
> -----Original Message-----
> From: Christopher Dolan [mailto:christopher.dolan@avid.com] 
> Sent: Tuesday, June 08, 2010 11:13 AM
> To: river-dev@incubator.apache.org
> Subject: RE: com.sun.jini.thread lock contention
>
> Oops, let me try that patch one more time...
> Chris
>
>
> -----Original Message-----
> From: Christopher Dolan 
> Sent: Tuesday, June 08, 2010 11:11 AM
> To: 'river-dev@incubator.apache.org'
> Subject: RE: com.sun.jini.thread lock contention
>
> Attached is a minimalist initial patch.  I'm not happy that I had to
> store a reference to the thread in the Runnable...  But otherwise I
> wasn't sure how to interrupt it from removeTask() without a separate map
> of TaskThread to Thread.  There's all a null pointer race in this patch
> if you try to remove a task before it's run() method executes.
>
> Chris
>
>   


Mime
View raw message