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

LocalFilesystemInteraction: Asynchronous read and write calls can cause reading an empty string from cache file #98

Open
RuesimOfCode opened this issue Sep 4, 2024 · 3 comments
Labels
Bug Something isn't working

Comments

@RuesimOfCode
Copy link

Bug Report

Q A
Version(s) 2.4.1

Summary

In our application multiple cron jobs running at the same time.

Circa every two hours (sporadically) we get an exception: Serialized data must be a string containing serialized PHP code; received:

Current behavior

The exception occurs because Laminas\Cache\Storage\Adapter\Filesystem::internalGetItem returns an empty string which cannot be deserialized by laminas/laminas-serializer/src/Adapter/PhpSerialize later on.

It turned out that cache files are created before filled and there is a short time slot where the file content is empty.
(the time slot is in Laminas\Cache\Storage\Adapter\Filesystem\LocalFilesystemInteraction::write after fopen and before flock)
Some micro- or milliseconds later the file is filled.

How to reproduce

  • Setup an application with laminas filesystem cache. The plugin serializer is enabled.
  • Add a debug break point to Laminas\Cache\Storage\Adapter\Filesystem\LocalFilesystemInteraction::write before flock is called the first time and after chmod is called (line 109 in 2.4.1).
  • Run into the break point with the debugger and keep the execution paused.
  • Disable debugger listening to the next request and ensure that the next request will not be blocked by the paused debug session.
  • Perform an asynchronous call that requests the same cache entry from filesystem cache.
  • Exception Serialized data must be a string containing serialized PHP code; received: should occur

I don't know if it would be possible to cover this case with a unit test (would require two asynchronous calls and a delay in chmod operation?)

Expected behavior

Laminas\Cache\Storage\Adapter\Filesystem::internalGetItem will always return with $success = false; or with the file content and never with empty content as a consequence of the file beeing created but the content not written yet (by Laminas\Cache\Storage\Adapter\Filesystem\LocalFilesystemInteraction::write).

Notes

  • The error would not occur on our system with the following workaround in Laminas\Cache\Storage\Adapter\Filesystem::internalGetItem before $success = true; is called:
            $maxIterations = 20; //max 20ms
            $currentIteration = 0;
            while ($data === '' && $currentIteration < $maxIterations) {
                usleep(1000); //1ms
                if (! $this->internalHasItem($normalizedKey)) {
                    $success = false;
                    return;
                }
                $data = $this->getFileContent($filespec);
                if ($data !== '') {
                    return $data;
                }
                $currentIteration++;
            }
  • Maybe there is a more reliable way to ensure that the created file will be filled when requested.
  • Maybe it would be possible to just return with $success = false; if the file content is empty and the serialize plugin is used (I don't know if an empty string is valid, if the serialize plugin is not used). However, without the serialize plugin an empty string could still be the result of a read/write bug.
@RuesimOfCode RuesimOfCode added the Bug Something isn't working label Sep 4, 2024
@RuesimOfCode
Copy link
Author

Update:

After testing different szeanarios we came to the conclustion that only two ways to create a cache file would solve the issue in LocalFilesystemInteraction's write method on our test environment:

  • file_put_contents with LOCK_EX flag
  • "fopen" a file with random suffix in filename and rename the file after "fclose" to the desired cache file name

As workaround we managed to inject a modified LocalFilesystemInteractionInteface to Filesystem. We implemented the solution with random suffix in filename of a temporary file in the "non-blocking" code path.

@Xerkus
Copy link
Member

Xerkus commented Sep 14, 2024

advisory file locks do not work across all file systems and they are just that - advisory. It is still possible to read incomplete file.

Best approach would be to do atomic writes via file move

@RuesimOfCode
Copy link
Author

Update: After using atomic writes via file move, we got another even rarer exception case.

After the check internalHasItem() returned true, it may happen that the cache file cannot be read, because it was deleted meanwhile.

So, the read is not atomic at the moment, too.

(Lately we switched to the 'redis' adapter.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants