apache / apache/jena

TransactionCoordinator's exclusivitylock isn't transferred by detach()/attach()

Open
#4,135 5 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
1.4k
Forks
712
Avg merge
15h 41m
Merged PRs (30d)
53

Description

### Version

6.2.0

### What happened?

TransactionCoordinator's exclusivitylock isn't transferred by detach()/attach(), so completing a transaction from a different thread than began it always throws IllegalMonitorStateException

## Version

6.2.0 (also present on `main` as of this report — the relevant code hasn't changed).

## Summary

`TransactionalSystem.detach()`/`attach(TransactionCoordinatorState)` (implemented by `TransactionalBase` in `jena-dboe-transaction`) are documented/intended to let a transaction be suspended off one thread and resumed on another. That works for the transaction's own `ThreadLocal` state and its components' per-thread state — but `TransactionCoordinator`'s "non-exclusive mode" lock is never transferred, so completing (`commit()`/`abort()`/`end()`) a transaction from any thread other than the one that called `begin()` throws unconditionally:

```
java.lang.IllegalMonitorStateException: attempt to unlock read lock, not locked by current thread
at java.util.concurrent.locks.ReentrantReadWriteLock$Sync.unmatchedUnlockException(ReentrantReadWriteLock.java:448)
at java.util.concurrent.locks.ReentrantReadWriteLock$Sync.tryReleaseShared(ReentrantReadWriteLock.java:432)
at java.util.concurrent.locks.AbstractQueuedSynchronizer.releaseShared(AbstractQueuedSynchronizer.java:1180)
at java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock.unlock(ReentrantReadWriteLock.java:897)
at org.apache.jena.dboe.transaction.txn.TransactionCoordinator.finishNonExclusiveMode(TransactionCoordinator.java:496)
at org.apache.jena.dboe.transaction.txn.TransactionCoordinator.finishActiveTransaction(TransactionCoordinator.java:1029)
at org.apache.jena.dboe.transaction.txn.TransactionCoordinator.completed(TransactionCoordinator.java:845)
at org.apache.jena.dboe.transaction.txn.Transaction.endInternal(Transaction.java:219)
at org.apache.jena.dboe.transaction.txn.Transaction.abort(Transaction.java:190)
at org.apache.jena.dboe.transaction.txn.TransactionalBase.abort(TransactionalBase.java:161)
```

## Steps to reproduce

```java
Dataset ds = TDB2Factory.connectDataset(Location.mem());
DatasetGraphTDB dsg = TDBInternal.getDatasetGraphTDB(ds);

Thread a = new Thread(() -> {
dsg.begin(TxnType.WRITE);
TransactionCoordinatorState state = dsg.getTxnSystem().detach();
// hand `state` off to another thread, e.g. via a queue/field
});
a.start(); a.join();

Thread b = new Thread(() -> {
dsg.getTxnSystem().attach(state); // resumes fine
dsg.getDefaultGraph().add(someTriple());
dsg.abort(); // <-- throws IllegalMonitorStateException here
dsg.end();
});
b.start(); b.join();
```

This is deterministic, not a race: it fails on the very first attempt to complete on a different thread, regardless of timing.

## Root cause

`TransactionCoordinator.java`:
- Line 100: `private ReadWriteLock exclusivitylock = new ReentrantReadWriteLock();` — comment at line 98: `// "one exclusive, or many other" lock which happens to be called ReadWriteLock`.
- Lines 486–492 (`tryNonExclusiveMode`): every ordinary transaction acquires the **read** side of this lock. Called from `begin(TxnType, boolean)` at line 661.
- Line 494: `finishNonExclusiveMode()` releases it via `exclusivitylock.readLock().unlock()`. Called from `finishActiveTransaction()` (line 1029), from `completed()` (line 845), from `Transaction.endInternal()` (`Transaction.java:213-219`) — i.e. on whatever thread finally commits/aborts/ends.

`java.util.concurrent.locks.ReentrantReadWriteLock`'s read lock tracks per-thread hold counts internally and throws `IllegalMonitorStateException` if a thread with a zero hold count calls `unlock()`. Because it's acquired on the `begin()`-calling thread and never transferred, any other thread that completes the transaction has a hold count of zero and crashes.

`TransactionCoordinator.detach(Transaction)`/`attach(TransactionCoordinatorState)` (lines 356–373) — the methods `TransactionalBase.detach()`/`attach()` delegate to — only call `txn.detach()`/`txn.attach()` and each component's `detach()`/`attach()`. Neither method references `exclusivitylock` at all. So the lock's thread affiliation is silently left pointing at the original `begin()` thread through any number of detach/attach cycles.

There is no thread-identity check or documentation anywhere in `Transaction.java`/`TransactionCoordinator.java` warning that `begin()` and completion must happen on the same thread; the constraint comes entirely from the JDK lock's own semantics, not from anything Jena manages.

## Why this looks like a real gap, not a misuse

A grep across the whole codebase (`jena-fuseki2`, `jena-tdb2`, `jena-arq`, `jena-db`) turns up no production call sites for `TransactionalSystem.attach()`/`detach()` outside the `dboe-transaction` module itself. The only test that exercises them (`jena-db/jena-dboe-transaction/src/test/java/org/apache/jena/dboe/transaction/TestTxnSwitching.java`) performs every `detach()`/`attach()` pair and the final `commit()`/`end()` on the single JUnit test thread. So the existing test suite can't have caught this: it never tries the "begin on thread A, complete on thread B" sequence that `detach()`/`attach()` seem designed to enable in the first place.

## Suggested fix

`exclusivitylock`'s read side is only ever used as a counting/blocking mechanism ("how many non-exclusive transactions are active, block them out for exclusive mode") — it doesn't need per-thread ownership semantics. Replacing it with a `java.util.concurrent.Semaphore` (already used elsewhere in this same class for `writersWaiting`, per the class's existing style) would make acquire/release thread-agnostic and fix this without touching `detach()`/`attach()`'s contract at all. Alternatively, `TransactionCoordinator.attach()` could explicitly transfer lock ownership (there's no public JDK API for that with `ReentrantReadWriteLock`, which is part of why a `Semaphore` seems like the simpler fix).

## Context

Found while prototyping a Fuseki module that holds a client-facing transaction across multiple HTTP requests (necessarily served by different threads from Jetty's pool) — see #4123 . Worked around there by pinning every operation for a given transaction to one dedicated thread for its whole lifetime, but that's a workaround, not a fix — the underlying `detach()`/`attach()` API can't actually do what its name suggests for anyone who wants completion to happen on a different thread than `begin()`.

- [ ] Are you interested in contributing a solution yourself? Possibly, if the `Semaphore` approach above sounds right to a maintainer — wanted to flag the root cause clearly first since it touches core transaction-coordination code I'd want sign-off on before proposing a specific patch.

### Relevant output and stacktrace

```shell

```

### Are you interested in making a pull request?

Maybe

Contributor guide

Open the contributing guide

Research direction

Start with TransactionCoordinator.java, especially detach()/attach(), tryNonExclusiveMode(), and finishNonExclusiveMode(), then inspect Transaction.java and TransactionalBase.java for the completion path. Run jena-db/jena-dboe-transaction/src/test/java/org/apache/jena/dboe/transaction/TestTxnSwitching.java and add coverage for beginning on one thread and completing on another. Done means cross-thread detach()/attach() completion no longer throws while existing exclusive-mode behavior remains intact.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend-api-design, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.