Skip to content

Fix Leaking Flocks#1469

Open
karnatmugo wants to merge 1 commit intoezsystems:masterfrom
mugoweb:ez/master
Open

Fix Leaking Flocks#1469
karnatmugo wants to merge 1 commit intoezsystems:masterfrom
mugoweb:ez/master

Conversation

@karnatmugo
Copy link

Context Of Problem

Lock Cleanup

When apache uses mod_php to serve a request, then any locks left open by PHP calls to flock are closed automatically when the request finishes.
This means that under normal use, buggy eZ PHP code that leaves dangling locks is harmless, as these are cleaned up.

However, for long running processes under gearman, this does not happen and dangling locks become dangerous.

eZ has a bug such that if object view cache is prepared using a template which sets cache_ttl to 0 will leave a dangling lock on the cache mutex.
A template doing this will have something like the following sample in it

{set-block scope=global variable=cache_ttl}0{/set-block}

This is in ezpublish_legacy/kernel/classes/clusterfilehandlers/ezfsfilehandler.php, near the end

        // Check if we are allowed to store the data, if not just return the result
        if ( !$store )
        {
            $this->abortCacheGeneration();
            return $result;
        }

        // Store content locally
        $this->storeContents( $binaryData, $scope, $datatype, true );

        $this->_freeExclusiveLock( 'storeCache' );

Here, if the cache ttl is set to zero, $store will be false, and the method will exit before it calls $this->_freeExclusiveLock.
If a lock has been taken, it will be leaked under gearman.  Under httpd mod_php or a command line php script, it will be cleaned at the end of the run.

    Context Of Problem

    Lock Cleanup

    When apache uses mod_php to serve a request, then any locks left open by PHP calls to flock are closed automatically when the request finishes.
    This means that under normal use, buggy eZ PHP code that leaves dangling locks is harmless, as these are cleaned up.

    However, for long running processes under gearman, this does not happen and dangling locks become dangerous.

    eZ has a bug such that if object view cache is prepared using a template which sets cache_ttl to 0 will leave a dangling lock on the cache mutex.
    A template doing this will have something like the following sample in it

    {set-block scope=global variable=cache_ttl}0{/set-block}

    This is in ezpublish_legacy/kernel/classes/clusterfilehandlers/ezfsfilehandler.php, near the end

            // Check if we are allowed to store the data, if not just return the result
            if ( !$store )
            {
                $this->abortCacheGeneration();
                return $result;
            }

            // Store content locally
            $this->storeContents( $binaryData, $scope, $datatype, true );

            $this->_freeExclusiveLock( 'storeCache' );

    Here, if the cache ttl is set to zero, $store will be false, and the method will exit before it calls $this->_freeExclusiveLock.
    If a lock has been taken, it will be leaked under gearman.  Under httpd mod_php or a command line php script, it will be cleaned at the end of the run.
@karnatmugo
Copy link
Author

@brookinsconsulting This attempts to address your comment on #1468

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 61 Bugs
Vulnerability E 4 Vulnerabilities
Security Hotspot E 4 Security Hotspots
Code Smell C 540 Code Smells

No Coverage information No Coverage information
33.8% 33.8% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant