[#UP-1016] BoundedThreadPool infinite loop or deadlock [2-4-patches]

uPortal

BoundedThreadPool infinite loop or deadlock [2-4-patches]

Created: 28/Apr/05 02:18 PM Updated: 03/Jul/07 04:11 PM

Component/s:

Rendering architecture

Affects Version/s:

2.0.3, 2.0, 2.1.1, 2.1.2, 2.1.4, 2.0.1, 2.0.2, 2.1.3, 2.1, 2.1.5, 2.2, 2.2.1, 2.3, 2.3.1, 2.3.2, 2.3.3, 2.3.4, 2.3.5, 2.4, 2.4.1, 2.4.2

Fix Version/s:

2.4.3

Original Estimate:

Unknown

Remaining Estimate:

Unknown

Time Spent:

Unknown

File Attachments:

backport-util-concurrent-patch.txt (289 kb)

Issue Links:

Generic Relation

This issue relates to:
	~~UP-931~~ ChannelRenderer finalizer is causing ...

This issue relates to:
	~~UP-862~~ BoundedThreadPool infinite loop or de...
	~~UP-1102~~ Restore safety to channel rendering

Description

« Hide

No one calls destroyThread() or stopWorker(). If someone were to call these methods, we'd either get deadlock or infinite recursion.

In looking through the Worker class in search of performance issues, Faizan and I noticed Worker.that stopWorker currently calls ThreadPool.destroyThread(this).

Currently the portal is using BoundedThreadPool and its destroyThread(Worker worker) implementation calls worker.stopWorker() which in turn calls destroyThread(this).

On top of that destroyThread is synchronized. There appears to be an opporunity here for either deadlock or StackOverflows (or whatever is thrown with those types of infinite loops) or both! Currently, stopWorker and destroyThread appear to call each other so this code is never actually executed. However, if executed accidentally it could have some adverse affects.

All

Comments

Work Log

Change History

Sort Order:

[ Permlink | « Hide ]

Andrew Petro [28/Apr/05 02:19 PM]

Cloning bug because this was resolved in HEAD but has not yet been resolved in 2-4-patches. So we need to track one still-open issue (with a fix-for of 2.4.3) and one resolved issue (with a fix-for of 2.5.whateveritwas).

[ Permlink | « Hide ]

Andrew Petro [28/Apr/05 02:22 PM]

I believe that under the changes from ~~UP-931~~, the broken method of BoundedThreadPool is now called, resulting in the StackOverflowError.

[ Permlink | « Hide ]

Andrew Petro [28/Apr/05 02:29 PM]

This issue is resolved in HEAD but not in 2-4-patches.

[ Permlink | « Hide ]

Brad Johnson [29/Apr/05 10:24 AM]

So are we okay with uPortal 2.4.2 as long as we don't modify some code to call destroyThread() or stopWorker()?

[ Permlink | « Hide ]

Faizan Ahmed [19/May/05 04:56 PM]

Replaced the home grown Thread pool (Bounded thread pool & unbounded thread pool) implementation with the stand alone thread pool (backport-util-concurrent) implementation.

Fix for ~~UP-1016~~
http://jasigch.princeton.edu:9000/jira/browse/UP-1016

Following files are Modified

portal/source
-org/javasig/portal
ChannelRenderer
ChannelRendererFactoryImpl
-org/javasig/portal/services
StatsRecorder

org/javasig/portal/services/stats
StatsRecorderWorkerTask** (19 files)
-build.xml
-build.properties

Newly added java files
-org/jasig/portal/utils/threading
BaseTask.java
RunnableTask.java
Task.java
PriorityThreadFactory.java

Following files has been Removed (10)
org/jasig/portal/utils/threading
-AbstractPool.java
-BoundedQueue.java
-BoundedThreadPool.java
-Queue.java
-ThreadPool.java
-UnboundedQueue.java
-UnboundedThreadPool.java
-Worker.java
-WorkerTask.java
-WorkTracker.java

Newly added jar

portal/WEB-INF/lib
backport-threadpool.jar

[ Permlink | « Hide ]

Faizan Ahmed [19/May/05 05:12 PM]

This patch intend to replace the bounded thread pool implementation with the backport-util-concurrent implementation.

Notes: This patch is created against the head on 05/09/2005 at about 3:15PM. Make sure your file versions match.
For some reason backport-util-concurrent.jar does not get copied properly and you will see some compiler error messages. Just copy manually backport-util-concurrent.jar into portal lib dir. Do not forget to include the bacport-util-concurrent.jar into your project class path.