[TDannhauer]

Summary

• Horde_ActiveSync_State_Sql no longer keeps a Horde_Db transaction open while the request hands control to the backend (imports, getSyncStamp(), getChanges()).
• Collection lock: the check-and-set transaction is committed on acquire; the committed token row is the lock. Release clears the token in autocommit mode and, if the connection carries an aborted/foreign transaction, rolls it back and retries once.
• State row: read FOR UPDATE in a short, immediately committed transaction; save() writes in its own short transaction. The _stateRowLockHeld / _stateRowLockTxnOwner / _collectionLockTxnOwner machinery is removed.

Fixes #109.

Motivation

With the lock transaction open across the whole Sync request, a single failed backend statement on PostgreSQL (observed: a Turba UPDATE rejected with SQLSTATE[22001]) aborted the shared transaction. Every following state query failed with SQLSTATE[25P02], the request returned HTTP 500, the client retried the same change on the next Sync, and the collection lock row could only be freed by the stale timeout because the token-clearing UPDATE failed too. In one deployment this produced 285 identical crashes in 18 days.

Horde_Db nests transactions by counter only (no savepoints), so holding a transaction across backend code is unsafe independent of PostgreSQL: a backend rollbackDbTransaction() in its own error handling would silently drop the ActiveSync lock rows and row lock.

Changes

lib/Horde/ActiveSync/State/Sql.php

• _acquireCollectionLock(): commitDbTransaction() right after the token UPDATE when this instance started the transaction.
• _releaseCollectionLock(): token clear in autocommit; on Horde_Db_Exception with transactionStarted(), log at warn, roll back via the existing _rollbackDbOnFailure(), retry the UPDATE once. The $commit parameter stays for the State_Base contract.
• _acquireStateRowLock() → _readStateRow(): same SELECT … FOR UPDATE, committed immediately after the read; StateGone is thrown after the commit.
• _saveSyncStateRow(): always own short transaction unless a caller already opened one.
• __destruct(), _loadState(), updateSyncStamp(): row-lock release calls removed.

Caught exception types in new/changed catch blocks: Horde_Db_Exception only — on the token-clear UPDATE (logged; triggers the rollback+retry when a transaction is open) and on the commit in _readStateRow() (logged, rethrown as Horde_ActiveSync_Exception).

Tests

• RowLockTest.php rewritten: read commits immediately, joins an open caller transaction without closing it, rolls back on query failure, commits before StateGone; save runs in its own transaction / joins the caller's / rolls back its own on failure; updateSyncStamp() below threshold touches no transaction.
• CollectionLockTest.php: acquire commits before returning (mock), save releases the lock in autocommit (mock), release rolls back an aborted transaction and retries (mock), SQLite acquire/release leaves no transaction open, and an end-to-end SQLite test where a backend statement fails between loadState() and save() — the save and lock release still succeed.
• ReadonlyLoadStateTest.php: asserts no transaction is open instead of the removed property.

Docs

• doc/architecture.md: new paragraph on the locking model and why no transaction may be open while the backend runs.
• doc/todo.md: stability list entry.

Not in scope: the Turba VARCHAR(10) postal-code columns that triggered the backend failure (separate issue).

Test plan

• phpunit test/unit/Horde/ActiveSync/StateTest — 68 tests pass
• Full test/unit suite: no new failures (4 pre-existing, unrelated failures in AppointmentTest, FindRequestTest, Rfc822Test fail identically on FRAMEWORK_6_0)
• PostgreSQL deployment: Sync of a Contacts collection with a backend write that fails continues, reports the item status, and no 25P02 follows in the log

[github-actions]

🔍 CI Results

Overall: ❌ 12/12 lanes failed

TL;DR: ❌ Quality issues: PHP-CS-Fixer: 46 files.

Summary by PHP Version

PHP	dev	stable
8.1	❌	❌
8.2	❌	❌
8.3	❌	❌
8.4	❌	❌
8.5	❌	❌
8.6	❌	❌

Quality Metrics

• PHPUnit: did not run (12 lanes exited non-zero with no test output) ❌
• PHPStan: No errors found ✅ — 8 advisories at level 2
• PHP-CS-Fixer: 46 unique files with issues (of 232 checked) ⚠️

❌ Failed Lanes

php8.1-dev

• PHPUnit: 0 failures, 0 errors

php8.1-stable

• PHPUnit: 0 failures, 0 errors

php8.2-dev

• PHPUnit: 0 failures, 0 errors

php8.2-stable

• PHPUnit: 0 failures, 0 errors

php8.3-dev

• PHPUnit: 0 failures, 0 errors

php8.3-stable

• PHPUnit: 0 failures, 0 errors

php8.4-dev

• PHPUnit: 0 failures, 0 errors
• PHP-CS-Fixer: 46 files with issues

php8.4-stable

• PHPUnit: 0 failures, 0 errors

php8.5-dev

• PHPUnit: 0 failures, 0 errors

php8.5-stable

• PHPUnit: 0 failures, 0 errors

php8.6-dev

• PHPUnit: 0 failures, 0 errors

php8.6-stable

• PHPUnit: 0 failures, 0 errors

---

CI powered by horde-components • View full results