ridljar/com/sun/star/lib/uno/environments/remote/JavaThreadPool.java |
14 ++
ridljar/com/sun/star/lib/uno/environments/remote/JavaThreadPoolFactory.java |
4
ridljar/com/sun/star/lib/uno/environments/remote/JobQueue.java |
60 ++++------
ridljar/test/com/sun/star/lib/uno/environments/remote/JobQueue_Test.java |
38 +++---
4 files changed, 63 insertions(+), 53 deletions(-)
New commits:
commit 132a8e19cfdabcefa22cb66bdae29a88a3e89793
Author: Stephan Bergmann <stephan.bergm...@collabora.com>
AuthorDate: Mon Jul 7 13:14:18 2025 +0200
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Tue Jul 8 16:14:33 2025 +0500
Java URP: Reliably check whether a JobQueue got disposed
If there are multiple Java URP bridges that get disposed simultaneously,
there
was a race in that one JobQueue could receive first its own dispose(), save
that
disposeId as _doDispose, then receive an unrelated dispose() and overwrite
its
_doDispose with that other disposeId, and only then get to the check of
_doDispose == _disposeId in removeJob(), which would thus never succeed and
cause the thread to hang in that loop forever.
So leverage the fact that the _disposeId, instead of being just an opaque
token,
actually is the JavaThreadPool that initiated the disposing, so can be used
to
check whether a JobQueue got disposed. (And rename some entities to make
their
meaning more obvious.)
(Which then required reworking parts of the test code that checks the
internals
of the Java URP bridge.)
Change-Id: If7f4f494b0f7e7f17e0a5c6a458582ad9bed3054
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187476
Reviewed-by: Stephan Bergmann <stephan.bergm...@collabora.com>
Tested-by: Jenkins
diff --git
a/ridljar/com/sun/star/lib/uno/environments/remote/JavaThreadPool.java
b/ridljar/com/sun/star/lib/uno/environments/remote/JavaThreadPool.java
index 332306be0e3d..21bb8ca9d0da 100644
--- a/ridljar/com/sun/star/lib/uno/environments/remote/JavaThreadPool.java
+++ b/ridljar/com/sun/star/lib/uno/environments/remote/JavaThreadPool.java
@@ -35,6 +35,8 @@ public class JavaThreadPool implements IThreadPool {
*/
private static final boolean DEBUG = false;
+ private Throwable _disposeThrowable = null;
+
JavaThreadPoolFactory _javaThreadPoolFactory;
JavaThreadPool(JavaThreadPoolFactory javaThreadPoolFactory) {
@@ -112,9 +114,19 @@ public class JavaThreadPool implements IThreadPool {
}
public void dispose(Throwable throwable) {
+ assert(throwable != null);
+
+ synchronized (this) {
+ _disposeThrowable = throwable;
+ }
+
if(DEBUG) System.err.println("##### " + getClass().getName() +
".dispose:" + throwable);
- _javaThreadPoolFactory.dispose(this, throwable);
+ _javaThreadPoolFactory.notifyAboutSomeDisposedPool();
+ }
+
+ public synchronized Throwable checkDisposed() {
+ return _disposeThrowable;
}
public void destroy() {
diff --git
a/ridljar/com/sun/star/lib/uno/environments/remote/JavaThreadPoolFactory.java
b/ridljar/com/sun/star/lib/uno/environments/remote/JavaThreadPoolFactory.java
index 181a3e17e40a..5c40cf61b1dc 100644
---
a/ridljar/com/sun/star/lib/uno/environments/remote/JavaThreadPoolFactory.java
+++
b/ridljar/com/sun/star/lib/uno/environments/remote/JavaThreadPoolFactory.java
@@ -52,14 +52,14 @@ final class JavaThreadPoolFactory {
return q == null ? null : q._async_jobQueue;
}
- public void dispose(Object disposeId, Throwable throwable) {
+ public void notifyAboutSomeDisposedPool() {
JobQueue[] qs;
synchronized (jobQueues) {
Collection<JobQueue> c = jobQueues.values();
qs = c.toArray(new JobQueue[c.size()]);
}
for (int i = 0; i < qs.length; ++i) {
- qs[i].dispose(disposeId, throwable);
+ qs[i].notifyAboutSomeDisposedPool();
}
}
diff --git a/ridljar/com/sun/star/lib/uno/environments/remote/JobQueue.java
b/ridljar/com/sun/star/lib/uno/environments/remote/JobQueue.java
index 2d4f3df6c939..25b9c48d8656 100644
--- a/ridljar/com/sun/star/lib/uno/environments/remote/JobQueue.java
+++ b/ridljar/com/sun/star/lib/uno/environments/remote/JobQueue.java
@@ -53,9 +53,7 @@ public class JobQueue {
private boolean _createThread_now; // create a worker thread, if needed
private Thread _worker_thread; // the thread that does the jobs
- private Object _disposeId; // the active dispose id
- private Object _doDispose = null;
- private Throwable _throwable;
+ private JavaThreadPool _pool; // the active pool
JobQueue _async_jobQueue; // chaining job queues for asyncs
protected JobQueue _sync_jobQueue; // chaining job queues for syncs
@@ -68,14 +66,14 @@ public class JobQueue {
* A thread for dispatching jobs.
*/
class JobDispatcher extends Thread {
- Object _disposeId;
+ JavaThreadPool _pool;
- JobDispatcher(Object disposeId) {
+ JobDispatcher(JavaThreadPool pool) {
super("JobDispatcher");
if(DEBUG) System.err.println("JobQueue$JobDispatcher.<init>:" +
_threadId);
- _disposeId = disposeId;
+ _pool = pool;
}
ThreadId getThreadId() {
@@ -87,7 +85,7 @@ public class JobQueue {
if(DEBUG) System.err.println("ThreadPool$JobDispatcher.run: " +
Thread.currentThread());
try {
- enter(2000, _disposeId);
+ enter(2000, _pool);
} catch(Throwable throwable) {
synchronized (JobQueue.this) {
if(!jobList.isEmpty() || _active) { // there was a job in
progress, so give a stack
@@ -196,10 +194,10 @@ public class JobQueue {
// wait max. waitTime time for a job to enter the queue
boolean waited = false;
while(jobList.isEmpty() && (waitTime == 0 || !waited)) {
- if(_doDispose == _disposeId) {
- _doDispose = null;
+ Throwable throwable = _pool.checkDisposed();
+ if(throwable != null) {
throw (DisposedException)
- new DisposedException().initCause(_throwable);
+ new DisposedException().initCause(throwable);
}
// notify sync queues
@@ -230,10 +228,10 @@ public class JobQueue {
while(_async_jobQueue._active ||
!_async_jobQueue.jobList.isEmpty()) {
if(DEBUG) System.err.println("waiting for async:" +
_async_jobQueue.jobList + " " + _async_jobQueue._worker_thread);
- if(_doDispose == _disposeId) {
- _doDispose = null;
+ Throwable throwable = _pool.checkDisposed();
+ if(throwable != null) {
throw (DisposedException)
- new DisposedException().initCause(_throwable);
+ new DisposedException().initCause(throwable);
}
try {
@@ -252,9 +250,9 @@ public class JobQueue {
* Puts a job into the queue.
*
* @param job the job.
- * @param disposeId a dispose id.
+ * @param pool a pool.
*/
- synchronized void putJob(Job job, Object disposeId) {
+ synchronized void putJob(Job job, JavaThreadPool pool) {
if(DEBUG) System.err.println("##### " + getClass().getName() +
".putJob todoes: " + " job:" + job);
jobList.add(job);
@@ -264,7 +262,7 @@ public class JobQueue {
acquire();
_createThread_now = false;
- new JobDispatcher(disposeId).start();
+ new JobDispatcher(pool).start();
}
// always notify possible waiters
@@ -274,29 +272,29 @@ public class JobQueue {
/**
* Enters the job queue.
*
- * @param disposeId a dispose id.
+ * @param pool a pool.
* @return the result of the final job (reply).
*/
- Object enter(Object disposeId) throws Throwable {
- return enter(0, disposeId); // wait infinitely
+ Object enter(JavaThreadPool pool) throws Throwable {
+ return enter(0, pool); // wait infinitely
}
/**
* Enters the job queue.
*
* @param waitTime the maximum amount of time to wait for a job (0
means wait infinitely).
- * @param disposeId a dispose id.
+ * @param pool a pool.
* @return the result of the final job (reply).
*/
- Object enter(int waitTime, Object disposeId) throws Throwable {
+ Object enter(int waitTime, JavaThreadPool pool) throws Throwable {
if(DEBUG) System.err.println("#####" + getClass().getName() + ".enter:
" + _threadId);
boolean quit = false;
- Object hold_disposeId;
+ JavaThreadPool hold_pool;
synchronized (this) {
- hold_disposeId = _disposeId;
- _disposeId = disposeId;
+ hold_pool = _pool;
+ _pool = pool;
}
Object result = null;
@@ -340,7 +338,7 @@ public class JobQueue {
_createThread_now = true;
- _disposeId = hold_disposeId;
+ _pool = hold_pool;
if(_sync_jobQueue != null)
notifyAll(); // notify waiters (e.g. this is an
asyncQueue and there is a sync waiting)
@@ -356,16 +354,11 @@ public class JobQueue {
}
/**
- * If the given disposeId is registered, interrupts the worker thread.
- *
- * @param disposeId the dispose id.
+ * Interrupts the worker thread so it can check whether its JavaThreadPool
got disposed.
*/
- synchronized void dispose(Object disposeId, Throwable throwable) {
+ synchronized void notifyAboutSomeDisposedPool() {
if(_sync_jobQueue == null) { // dispose only sync queues
- _doDispose = disposeId;
- _throwable = throwable;
-
- // get thread out of wait and let it throw the throwable
+ // get thread out of wait
if(DEBUG) System.err.println(getClass().getName() + ".dispose -
notifying thread");
notifyAll();
diff --git
a/ridljar/test/com/sun/star/lib/uno/environments/remote/JobQueue_Test.java
b/ridljar/test/com/sun/star/lib/uno/environments/remote/JobQueue_Test.java
index a63c9c7ed6ba..76f8fe76beef 100644
--- a/ridljar/test/com/sun/star/lib/uno/environments/remote/JobQueue_Test.java
+++ b/ridljar/test/com/sun/star/lib/uno/environments/remote/JobQueue_Test.java
@@ -42,7 +42,8 @@ public final class JobQueue_Test {
TestThread t = new TestThread(waitTime);
t.waitToStart();
String msg = "xcxxxxxxxx";
- t._jobQueue.dispose(t._disposeId, new RuntimeException (msg));
+ t._pool.dispose(new RuntimeException (msg));
+ t._jobQueue.notifyAboutSomeDisposedPool();
t.waitToTerminate();
/*TODO: below test fails with "expected:<xcxxxxxxxx> but was:<null>":
assertEquals(msg, t._message);
@@ -94,9 +95,9 @@ public final class JobQueue_Test {
{
TestThread t = new TestThread(waitTime);
t.waitToStart();
- testExecuteJobs(t._jobQueue);
- t._jobQueue.dispose(t._disposeId,
- new RuntimeException("xxxxxxxxxxxxx"));
+ testExecuteJobs(t._jobQueue, t._pool);
+ t._pool.dispose(new RuntimeException("xxxxxxxxxxxxx"));
+ t._jobQueue.notifyAboutSomeDisposedPool();
t.waitToTerminate();
}
@@ -104,7 +105,8 @@ public final class JobQueue_Test {
{
testExecuteJobs(
new JobQueue(
- __javaThreadPoolFactory, ThreadId.createFresh(), true));
+ __javaThreadPoolFactory, ThreadId.createFresh(), true),
+ new JavaThreadPool(__javaThreadPoolFactory));
}
@Test public void testStaticThreadExecutesAsyncs()
@@ -118,9 +120,9 @@ public final class JobQueue_Test {
assertEquals(1, t._jobQueue._ref_count);
t.waitToStart();
TestWorkAt workAt = new TestWorkAt();
- testAsyncJobQueue(workAt, async_jobQueue, t._threadId);
- t._jobQueue.dispose(t._disposeId,
- new RuntimeException("xxxxxxxxxxxxx"));
+ testAsyncJobQueue(workAt, async_jobQueue, t._threadId, t._pool);
+ t._pool.dispose(new RuntimeException("xxxxxxxxxxxxx"));
+ t._jobQueue.notifyAboutSomeDisposedPool();
t.waitToTerminate();
assertEquals(TestWorkAt.MESSAGES, workAt._async_counter);
assertEquals(TestWorkAt.MESSAGES, workAt._sync_counter);
@@ -133,15 +135,15 @@ public final class JobQueue_Test {
JobQueue async_jobQueue = new JobQueue(__javaThreadPoolFactory,
threadId);
TestWorkAt workAt = new TestWorkAt();
- testAsyncJobQueue(workAt, async_jobQueue, threadId);
+ testAsyncJobQueue(workAt, async_jobQueue, threadId, new
JavaThreadPool(__javaThreadPoolFactory));
assertEquals(TestWorkAt.MESSAGES, workAt._async_counter);
assertEquals(TestWorkAt.MESSAGES, workAt._sync_counter);
}
- private void testExecuteJobs(JobQueue jobQueue) throws InterruptedException
+ private void testExecuteJobs(JobQueue jobQueue, JavaThreadPool pool)
throws InterruptedException
{
TestWorkAt workAt = new TestWorkAt();
- testSendRequests(workAt, "increment", jobQueue);
+ testSendRequests(workAt, "increment", jobQueue, pool);
synchronized (workAt) {
jobQueue.putJob(new Job(workAt, __iReceiver,
new Message(
@@ -158,13 +160,13 @@ public final class JobQueue_Test {
}
private void testAsyncJobQueue(TestWorkAt workAt, JobQueue async_jobQueue,
- ThreadId threadId)
+ ThreadId threadId, JavaThreadPool pool)
throws InterruptedException
{
// put slow async calls first, followed by fast sync calls:
- testSendRequests(workAt, "asyncCall", async_jobQueue);
+ testSendRequests(workAt, "asyncCall", async_jobQueue, pool);
testSendRequests(workAt, "syncCall",
- __javaThreadPoolFactory.getJobQueue(threadId));
+ __javaThreadPoolFactory.getJobQueue(threadId), pool);
synchronized (workAt) {
async_jobQueue._sync_jobQueue.putJob(
new Job(workAt, __iReceiver,
@@ -181,7 +183,7 @@ public final class JobQueue_Test {
}
private void testSendRequests(TestWorkAt workAt, String operation,
- JobQueue jobQueue) {
+ JobQueue jobQueue, JavaThreadPool pool) {
Message iMessage = new Message(
null, true, "oid", __workAt_td,
__workAt_td.getMethodDescription(operation),
@@ -189,13 +191,13 @@ public final class JobQueue_Test {
for (int i = 0; i < TestWorkAt.MESSAGES; ++ i) {
Thread.yield(); // force scheduling
jobQueue.putJob(new Job(workAt, __iReceiver, iMessage),
- new Object());
+ pool);
}
}
private static final class TestThread extends Thread {
public final ThreadId _threadId = JavaThreadPoolFactory.getThreadId();
- public final Object _disposeId = new Object();
+ public final JavaThreadPool _pool = new
JavaThreadPool(__javaThreadPoolFactory);
public JobQueue _jobQueue = null;
public TestThread(int waitTime) {
@@ -217,7 +219,7 @@ public final class JobQueue_Test {
if (waitTime != 0) {
Thread.sleep(waitTime);
}
- _jobQueue.enter(_disposeId);
+ _jobQueue.enter(_pool);
} catch (Throwable e) {
}
synchronized (lock) {
commit 10d421f8ba3d1c385d77f9b04dd0fee3fdd2b7b2
Author: Stephan Bergmann <stephan.bergm...@collabora.com>
AuthorDate: Mon Jul 7 13:24:08 2025 +0200
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Tue Jul 8 16:14:33 2025 +0500
Consistently synchronize access to JobQueue._disposeId
Change-Id: I4a6700a519f65c825ca310caa100e3be2e0562ac
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187477
Reviewed-by: Stephan Bergmann <stephan.bergm...@collabora.com>
Tested-by: Jenkins
diff --git a/ridljar/com/sun/star/lib/uno/environments/remote/JobQueue.java
b/ridljar/com/sun/star/lib/uno/environments/remote/JobQueue.java
index b71525ba5dd8..2d4f3df6c939 100644
--- a/ridljar/com/sun/star/lib/uno/environments/remote/JobQueue.java
+++ b/ridljar/com/sun/star/lib/uno/environments/remote/JobQueue.java
@@ -293,8 +293,11 @@ public class JobQueue {
boolean quit = false;
- Object hold_disposeId = _disposeId;
- _disposeId = disposeId;
+ Object hold_disposeId;
+ synchronized (this) {
+ hold_disposeId = _disposeId;
+ _disposeId = disposeId;
+ }
Object result = null;
