Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Leaking Flocks #1469

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix Leaking Flocks #1469

wants to merge 1 commit into from

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

@sonarcloud
Copy link

sonarcloud bot commented Aug 18, 2023

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