TreeBehavior calculates wrong values for lft/rght when called concurrently #17174
Description
Description
Hi,
i have noticed a problematic behavior (bug?) within the TreeBehavior when concurrent requests are used. In my case, CakePHP is used to provide the backend API to the frontend, and the frontend can send multiple concurrent requests to the same API endpoint.
This leads to all sort of random issues like having multiple nodes with the same values for lft
and rght
, or nodes where lft
is greater-than than rght
.
I was only able to reproduce this while deleting nodes. I'm not sure why to be honest. This should also happen when creating new nodes.
To prevent this issue, some sort of locking is required. I decided to go with a Semaphor sem_get.
My plan was to implement the locking via the beforeDelete
and afterDelete
callbacks of the table, where I noticed the next bug.
It looks like, that the TreeBehavior is not calling the beforeSave
and afterSave
events correctly when the TreeBehavior
has to updated existing nodes.
So, I still have the same issue, when I implement the locking like so:
<?php
namespace App\Model\Table;
class ContainersTable extends Table {
public function initialize(array $config): void {
parent::initialize($config);
$this->setTable('containers');
$this->setDisplayField('name');
$this->setPrimaryKey('id');
$this->addBehavior('Tree', [
'cascadeCallbacks' => true // I tried true and false - same results
]);
}
public function beforeSave(EventInterface $event, EntityInterface $entity, \ArrayObject $options) {
Log::debug('beforeSave - Acquire table lock');
$this->acquireLock();
}
public function afterSave(EventInterface $event, EntityInterface $entity, \ArrayObject $options) {
Log::debug(' afterSave - Release table lock');
$this->releaseLock();
}
public function beforeDelete(EventInterface $event, EntityInterface $entity, \ArrayObject $options) {
Log::debug('beforeDelete - Acquire table lock');
$this->acquireLock();
}
public function afterDelete(EventInterface $event, EntityInterface $entity, \ArrayObject $options) {
Log::debug('afterDelete - Release table lock');
$this->releaseLock();
}
/**
* Will create a Semaphor with a limit of 1 to avoid multiple operations running concurrently on the containers table
* This method blocks (if necessary) until the semaphore can be acquired
*
* https://github.com/cakephp/cakephp/issues/14983#issuecomment-1613491168
* @return bool
*/
public function acquireLock() {
if ($this->semaphoreKey === null) {
$this->semaphoreKey = ftok(__FILE__, 'c');
$this->semaphore = sem_get($this->semaphoreKey, 1, 0666, true);
}
return sem_acquire($this->semaphore);
}
/**
* @return bool
*/
public function releaseLock() {
if (!is_null($this->semaphore)) {
return sem_release($this->semaphore);
}
}
}
The locking works as expected, when im implement it inside the Controller:
<?php
namespace App\Controller;
class ServicetemplategroupsController extends AppController {
public function delete($id = null) {
/** @var ContainersTable $ContainersTable */
$ContainersTable = TableRegistry::getTableLocator()->get('Containers');
$ContainersTable->acquireLock();
/** @var $ContainersTable ContainersTable */
$ContainersTable = TableRegistry::getTableLocator()->get('Containers');
$container = $ContainersTable->get($id, [
'contain' => [
'Servicetemplategroups'
]
]);
$ContainersTable->delete($container);
$ContainersTable->releaseLock();
}
}
But this means I have to dig through all the code to find all parts that are modifying the tree.
Edit: There was also an issue in the past with the same problem: #14983
Update
Looks like the tree behavior is calling the _sync
method, before the beforeDelete
method in the Table gets executed
CakePHP Version
4.4.14
PHP Version
8.1