Skip to content

TreeBehavior calculates wrong values for lft/rght when called concurrently #17174

Open
@nook24

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.

broke_tree

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 beforeSaveand 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

tree_sync

CakePHP Version

4.4.14

PHP Version

8.1

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions