Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 51 additions & 12 deletions lib/Horde/ActiveSync/Collections.php
Original file line number Diff line number Diff line change
Expand Up @@ -788,9 +788,9 @@
if (isset($folder->serverid)
&& $syncFolder = $this->_cache->getFolder($folder->serverid)
&& in_array($folder->serverid, $seenFolders)
&& $syncFolder['parentid'] == $folder->parentid

Check failure on line 791 in lib/Horde/ActiveSync/Collections.php

View workflow job for this annotation

GitHub Actions / CI

PHPStan level 1

Variable $syncFolder might not be defined. [variable.undefined]
&& $syncFolder['displayname'] == $folder->displayname

Check failure on line 792 in lib/Horde/ActiveSync/Collections.php

View workflow job for this annotation

GitHub Actions / CI

PHPStan level 1

Variable $syncFolder might not be defined. [variable.undefined]
&& $syncFolder['type'] == $folder->type) {

Check failure on line 793 in lib/Horde/ActiveSync/Collections.php

View workflow job for this annotation

GitHub Actions / CI

PHPStan level 1

Variable $syncFolder might not be defined. [variable.undefined]

$this->_logger->meta(
sprintf(
Expand Down Expand Up @@ -910,8 +910,8 @@
$this->save();
return false;
}
$this->shortSyncRequest = true;

Check failure on line 913 in lib/Horde/ActiveSync/Collections.php

View workflow job for this annotation

GitHub Actions / CI

PHPStan level 1

Access to an undefined property Horde_ActiveSync_Collections::$shortSyncRequest. [property.notFound]
$this->hangingSync = true;

Check failure on line 914 in lib/Horde/ActiveSync/Collections.php

View workflow job for this annotation

GitHub Actions / CI

PHPStan level 1

Access to an undefined property Horde_ActiveSync_Collections::$hangingSync. [property.notFound]
$this->save(true);

return true;
Expand Down Expand Up @@ -1273,7 +1273,7 @@

// Need to update AND SAVE the timestamp for race conditions to be
// detected.
$this->lasthbsyncstarted = $started;

Check failure on line 1276 in lib/Horde/ActiveSync/Collections.php

View workflow job for this annotation

GitHub Actions / CI

PHPStan level 1

Access to an undefined property Horde_ActiveSync_Collections::$lasthbsyncstarted. [property.notFound]
$this->save();

// We only check for remote wipe request once every 5 iterations to
Expand Down Expand Up @@ -1337,12 +1337,25 @@
$e->getMessage()
)
);
$this->_as->state->loadState(
[],
null,
Horde_ActiveSync::REQUEST_TYPE_SYNC,
$id
);
try {
$this->_as->state->loadState(
[],
null,
Horde_ActiveSync::REQUEST_TYPE_SYNC,
$id
);
} catch (Horde_ActiveSync_Exception_TemporaryFailure $e) {
// Reset blocked by a parallel request holding the
// collection lock. Retry on the next poll iteration.
$this->_logger->notice(
sprintf(
'COLLECTIONS: State reset for %s deferred: %s',
$id,
$e->getMessage()
)
);
continue;
}
$this->setGetChangesFlag($id);
$dataavailable = true;
continue;
Expand Down Expand Up @@ -1394,6 +1407,19 @@
} catch (Horde_ActiveSync_Exception_FolderGone $e) {
$this->_logger->warn('COLLECTIONS: Folder gone for collection ' . $collection['id']);
return self::COLLECTION_ERR_FOLDERSYNC_REQUIRED;
} catch (Horde_ActiveSync_Exception_TemporaryFailure $e) {
// Transient contention (e.g. collection lock held by a
// parallel SYNC). Skip this collection for now; it will
// be retried on the next poll iteration. Do NOT reset
// state for a transient condition.
$this->_logger->notice(
sprintf(
'COLLECTIONS: Temporary failure loading state for %s; retrying next iteration: %s',
$id,
$e->getMessage()
)
);
continue;
} catch (Horde_ActiveSync_Exception $e) {
$this->_logger->err('COLLECTIONS: Error loading state: ' . $e->getMessage());
$this->_as->state->loadState(
Expand Down Expand Up @@ -1437,12 +1463,25 @@
$e->getMessage()
)
);
$this->_as->state->loadState(
[],
null,
Horde_ActiveSync::REQUEST_TYPE_SYNC,
$id
);
try {
$this->_as->state->loadState(
[],
null,
Horde_ActiveSync::REQUEST_TYPE_SYNC,
$id
);
} catch (Horde_ActiveSync_Exception_TemporaryFailure $e) {
// Reset blocked by a parallel request holding the
// collection lock. Retry on the next poll iteration.
$this->_logger->notice(
sprintf(
'COLLECTIONS: State reset for %s deferred: %s',
$id,
$e->getMessage()
)
);
continue;
}
$this->setGetChangesFlag($id);
$dataavailable = true;
} catch (Horde_ActiveSync_Exception_FolderGone $e) {
Expand Down Expand Up @@ -1498,7 +1537,7 @@
sprintf(
'COLLECTIONS: Looping Sync complete: DataAvailable: %s, DataImported: %s',
$dataavailable,
$this->importedChanges

Check failure on line 1540 in lib/Horde/ActiveSync/Collections.php

View workflow job for this annotation

GitHub Actions / CI

PHPStan level 1

Access to an undefined property Horde_ActiveSync_Collections::$importedChanges. [property.notFound]
)
);

Expand Down
15 changes: 11 additions & 4 deletions lib/Horde/ActiveSync/State/Base.php
Original file line number Diff line number Diff line change
Expand Up @@ -1115,10 +1115,17 @@ public function loadState(array $collection, $syncKey, $type = null, $id = null)
if ($type == Horde_ActiveSync::REQUEST_TYPE_FOLDERSYNC) {
$this->_folder = [];
} else {
// Create a new folder object.
$this->_folder = ($this->_collection['class'] == Horde_ActiveSync::CLASS_EMAIL)
? new Horde_ActiveSync_Folder_Imap($this->_collection['serverid'], Horde_ActiveSync::CLASS_EMAIL)
: ($this->_collection['serverid'] == 'RI' ? new Horde_ActiveSync_Folder_RI('RI', 'RI') : new Horde_ActiveSync_Folder_Collection($this->_collection['serverid'], $this->_collection['class']));
// Create a new folder object. The collection array may be
// empty when resetting state during error recovery.
$class = $this->_collection['class'] ?? null;
$serverid = $this->_collection['serverid'] ?? null;
if ($class == Horde_ActiveSync::CLASS_EMAIL) {
$this->_folder = new Horde_ActiveSync_Folder_Imap($serverid, Horde_ActiveSync::CLASS_EMAIL);
} elseif ($serverid == 'RI') {
$this->_folder = new Horde_ActiveSync_Folder_RI('RI', 'RI');
} else {
$this->_folder = new Horde_ActiveSync_Folder_Collection($serverid, $class);
}
}
$this->_syncKey = '0';
$lockFolderId = ($type == Horde_ActiveSync::REQUEST_TYPE_FOLDERSYNC)
Expand Down
4 changes: 3 additions & 1 deletion lib/Horde/ActiveSync/State/Mongo.php
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,9 @@ protected function _acquireCollectionLock($folderId = null)
}
}

throw new Horde_ActiveSync_Exception('Could not acquire collection lock.');
// Contention with a parallel request is a transient condition, not
// corrupt state - callers must not react with a state reset.
throw new Horde_ActiveSync_Exception_TemporaryFailure('Could not acquire collection lock.');
}

/**
Expand Down
5 changes: 4 additions & 1 deletion lib/Horde/ActiveSync/State/Sql.php
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,10 @@ protected function _acquireCollectionLock($folderId = null)
if ($started) {
$this->_db->rollbackDbTransaction();
}
throw new Horde_ActiveSync_Exception('Collection lock held.');
// Contention with a parallel request is a transient
// condition, not corrupt state - callers must not react
// with a state reset.
throw new Horde_ActiveSync_Exception_TemporaryFailure('Collection lock held.');
}

$token = random_int(1, PHP_INT_MAX);
Expand Down
93 changes: 93 additions & 0 deletions test/unit/Horde/ActiveSync/PingCollectionLockContentionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
<?php

/**
* Unit tests for PING behavior when the collection lock is held by a
* parallel request.
*
* Copyright 2026 The Horde Project (http://www.horde.org/)
*
* @author Torben Dannhauer <torben@dannhauer.de>
* @license http://www.horde.org/licenses/gpl GPLv2
* @copyright 2026 The Horde Project
* @package ActiveSync
*/

namespace Horde\ActiveSync;

use Horde_ActiveSync;
use Horde_ActiveSync_Collections;
use Horde_ActiveSync_Exception_TemporaryFailure;
use Horde_ActiveSync_Log_Logger;
use Horde_ActiveSync_SyncCache;
use Horde_Log_Handler_Null;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\TestCase;
use ReflectionProperty;

#[CoversClass(Horde_ActiveSync_Collections::class)]
class PingCollectionLockContentionTest extends TestCase
{
private const COLLECTION_ID = 'F1234abcd';

/**
* A collection lock held by a parallel SYNC must not trigger a state
* reset; the collection is skipped and retried on the next iteration.
*/
public function testPollForChangesSkipsCollectionOnLockContention()
{
$state = $this->createMock('Horde_ActiveSync_State_Sql');
$state->method('getSyncCache')->willReturn(['timestamp' => 0]);
$state->expects($this->never())->method('loadState');

$cache = new Horde_ActiveSync_SyncCache($state, 'device', 'user');

$as = $this->createMock(Horde_ActiveSync::class);
$as->logger = new Horde_ActiveSync_Log_Logger(new Horde_Log_Handler_Null());
$as->state = $state;
$as->device = (object) [
'id' => 'device',
'version' => Horde_ActiveSync::VERSION_FOURTEEN,
];
$as->provisioning = Horde_ActiveSync::PROVISIONING_NONE;

$collections = $this->getMockBuilder(Horde_ActiveSync_Collections::class)
->setConstructorArgs([$cache, $as])
->onlyMethods([
'initCollectionState',
'updateCollectionsFromCache',
'checkStaleRequest',
'collectionsNeedFolderResync',
'restorePingableCollectionsFromCache',
'havePingableCollections',
'haveHierarchy',
'save',
'setGetChangesFlag',
'_sleep',
])
->getMock();

$collections->method('collectionsNeedFolderResync')->willReturn(false);
$collections->method('havePingableCollections')->willReturn(true);
$collections->method('haveHierarchy')->willReturn(true);
$collections->method('checkStaleRequest')->willReturn(false);
$collections->method('initCollectionState')->willThrowException(
new Horde_ActiveSync_Exception_TemporaryFailure('Collection lock held.')
);
$collections->expects($this->never())->method('setGetChangesFlag');

$property = new ReflectionProperty(Horde_ActiveSync_Collections::class, '_collections');
$property->setAccessible(true);
$property->setValue($collections, [
self::COLLECTION_ID => [
'id' => self::COLLECTION_ID,
'synckey' => '{6a13541c-a3bc-448b-853c-915b00000000}14',
'class' => Horde_ActiveSync::CLASS_EMAIL,
'serverid' => 'INBOX',
],
]);

$this->assertFalse(
$collections->pollForChanges(1, 1, ['pingable' => true])
);
}
}
67 changes: 67 additions & 0 deletions test/unit/Horde/ActiveSync/StateTest/Sql/CollectionLockTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,24 @@
use Horde\ActiveSync\Test\Helpers\DbHelper;
use PHPUnit\Framework\TestCase;
use Horde_ActiveSync;
use Horde_ActiveSync_Exception_TemporaryFailure;
use Horde_ActiveSync_Folder_Collection;
use Horde_ActiveSync_State_Sql;
use Horde_ActiveSync_Folder_Imap;
use Horde_ActiveSync_Log_Logger;
use Horde_Db_Adapter_Pdo_Sqlite;
use Horde_Log_Handler_Null;
use ReflectionClass;

/**
* tables() is provided by the schema object via __call() on real adapters,
* so it must be declared explicitly for mocking.
*/
interface DbAdapterWithTables extends \Horde_Db_Adapter
{
public function tables();
}

class CollectionLockTest extends TestCase
{
public function testAcquireAndReleaseCollectionLockOnSqlite()
Expand Down Expand Up @@ -84,6 +95,62 @@ public function testSaveCommitsCollectionLockTransaction()
$this->assertFalse($this->_getProperty($state, '_stateRowLockHeld'));
}

public function testHeldLockThrowsTemporaryFailure()
{
$db = $this->createMock(DbAdapterWithTables::class);
$db->method('tables')->willReturn(['horde_activesync_collection_lock']);
$db->method('transactionStarted')->willReturn(false);
$db->method('selectValue')->willReturn(1);
$db->method('selectOne')->willReturn([
'lock_token' => 42,
'lock_time' => time(),
]);
$db->expects($this->atLeastOnce())->method('rollbackDbTransaction');

$state = $this->_newState($db);
$this->_setProperty($state, '_type', Horde_ActiveSync::REQUEST_TYPE_SYNC);
$this->_setProperty($state, '_collection', ['id' => 'Ftest']);

$this->expectException(Horde_ActiveSync_Exception_TemporaryFailure::class);
$this->_method($state, '_acquireCollectionLock')->invoke($state);
}

public function testLoadStateResetWithEmptyCollectionEmitsNoWarnings()
{
if (!extension_loaded('pdo_sqlite')) {
$this->markTestSkipped('PDO SQLite extension is not loaded');
}

$migrationDir = dirname(__DIR__, 6) . '/migration/Horde/ActiveSync';
$db = DbHelper::createSqliteDb([
'migrations' => [[
'migrationsPath' => $migrationDir,
'schemaTableName' => 'horde_activesync_schema_info',
]],
]);

$state = $this->_newState($db);

$warnings = [];
set_error_handler(function ($errno, $errstr) use (&$warnings) {
$warnings[] = $errstr;
return true;
}, E_WARNING | E_NOTICE);
try {
// Error-recovery resets pass an empty collection array.
$state->loadState([], null, Horde_ActiveSync::REQUEST_TYPE_SYNC, 'Ftest');
} finally {
restore_error_handler();
}

$this->assertSame([], $warnings);
$this->assertInstanceOf(
Horde_ActiveSync_Folder_Collection::class,
$this->_getProperty($state, '_folder')
);
$this->assertFalse($this->_getProperty($state, '_collectionLockHeld'));
}

public function testSkipsCollectionLockWhenTableMissing()
{
if (!extension_loaded('pdo_sqlite')) {
Expand Down
Loading