krakjoe / apcu

APCu - APC User Cache
Other
959 stars 194 forks source link

Apparent concurrency issue on PHP 8.1.6 in Windows #445

Closed nexussys closed 1 year ago

nexussys commented 2 years ago

I'm using APCu on Windows using IIS and Fast-CGI. APCu used to not work for me in PHP 7 (crashes all the time), and then I read it was fixed in PHP 8, just in time since WinCache is gone. It's been working mostly fine, but recently I noticed that if I update the cache using apcu_store while there are simultaneous reads using apcu_fetch, the cache fails to update. Or better yet, it seems to update and then rolls back to the previous value. To replicate, for lack of a more elegant solution, I opened two tabs in Chrome, one running code to fetch the key in a loop, the other to store a value. So the code looks like this:

Store Code

$key = 'test_var';
apcu_delete($key);
apcu_store($key, $_GET['val']);

Fetch code

$key = 'test_var';
for($i=0; $i<10000000; $i++) {
    $val = apcu_fetch($key, $success);
}
var_dump($success, $val);

I run the store code once passing in val as a certain value, such as 1. After, I run the fetch code in another tab, then immediately after run the store code again passing in another value (such as 2). When the fetch code returns, it still shows the initial value. If I check apc.php file during the operation, I can see the value changes to 2 after the store operation, and the Last Modified/Created At dates change. But then once the fetch code completes, the value moves back to the old value and the Last Modified/Created dates also roll back.

Any idea how to solve this problem?

nikic commented 2 years ago

What SAPI are you using? The APCu cache is per-process on Windows, so if you are using a process-based (rather than thread-based) SAPI, then it will not be shared between different processes. (This should really get documented somewhere...)

nexussys commented 2 years ago

Not using SAPI, using FastCGI, but I believe it behaves the same, multiple processes. However, it used to not work at all before recently when I started reading reports that it was working. This is the only issue I've run into, other than that it's worked pretty well. Before, it simply crashed all the time.

TysonAndre commented 1 year ago

What's the value of PHP_ZTS (full phpinfo section for php would also be useful)

I assume you have it enabled if they're sharing memory somehow

php > echo PHP_ZTS;
0
TysonAndre commented 1 year ago

https://github.com/krakjoe/apcu/blob/47c9f388920481138459c30daadc96b1e6343372/apc_shm.c#L48-L82

It's using shmget/shmat - maybe the behavior of those functions changed for processes

#ifdef PHP_WIN32
/* shm functions are available in TSRM */
#include <tsrm/tsrm_win32.h>
TysonAndre commented 1 year ago

So I seem to have been mistaken about php processes being able to share memory on windows.

https://github.com/php/php-src/pull/8648 changed in May in php 8.1.

As you've stated, the fix (for a different bug) was in php 8.1.6 but not 8.1.5 and earlier

I'm not really familiar with that code or the windows apis, though, it may be an improvement that requires extra locking, or it might be impossible to use with apcu (haven't checked: maybe forking tsrm_win32 for apcu would solve this?)

To share data, multiple processes can use memory-mapped files that the system paging file stores.

https://learn.microsoft.com/en-us/windows/win32/memory/creating-named-shared-memory - I haven't read this but it seems useful

cmb69 commented 1 year ago

The point is that APCu allocates private SHMs:

https://github.com/krakjoe/apcu/blob/98b26169d4145fef67df9e54e28f8af0720a5cdf/apc_shm.c#L52-L55

These private SHMs are not supposed to be shared between different processes (unless that would be forked child processes), but with classic (F)CGI the processes are different (not Windows specific, but on other systems, people likely use FPM). The relevant behavioral change is due to fixing bug 79566, which actually implemented IPC_PRIVATE on Windows; that bug is fixed as of PHP 7.3.19 and 7.4.7, though. The crashes on PHP 7, reported by @nexussys, might be caused by the non-private SHMs, which likely require additional locking.

So, as is, this is expected behavior, although it makes APCu practically useless for (F)CGI. So either WONTFIX + documentation improvements, or if desired, I could have a closer look on how it would be possible to support classic (F)CGI.

PS: All good with php8apache2_4.dll.

cmb69 commented 1 year ago

https://learn.microsoft.com/en-us/windows/win32/memory/creating-named-shared-memory - I haven't read this but it seems useful

This is already done by the portability implemention of shmget() (unless IPC_PRIVATE is requested).

TysonAndre commented 1 year ago

The point is that APCu allocates private SHMs:

To clarify for anyone wondering, that's only on windows (#ifdef PHP_WIN32, which is defined on all windows architectures), which doesn't have mmap support. On other OSes, it uses mmap (EDIT: uses mmap by default, but there is a configuration option to switch to shm)

https://github.com/krakjoe/apcu/blob/98b26169d4145fef67df9e54e28f8af0720a5cdf/apc_shm.c#L31

TysonAndre commented 1 year ago

I'm not a maintainer of apcu, but I believe IPC_PRIVATE is intentional and it's reasonable to stay that way and better that it's working (it should remain among processes in the same process group in general, I believe changing this would cause issues (e.g. with incompatible php versions)). e.g. ext/opcache/shared_alloc_shm.c also uses IPC_PRIVATE.

This can probably be closed; I believe that having different processes on Windows with fastcgi have different apcu shared memory caches is expected, and the cache you get will depend on which process it gets sent to. The cache is still useful, because the same process will be used for processing hundreds or thousands or more http requests, depending on the configuration)

(my concern was if processes accidentally polluted other processes' shared cache due to a shmget implementation change, but this appears not to be the case, especially with opcache doing the same thing)

cmb69 commented 1 year ago

To clarify for anyone wondering, that's only on windows (#ifdef PHP_WIN32, which is defined on all windows architectures), which doesn't have mmap support. On other OSes, it uses mmap.

That appears to be configurable on non Windows architectures:

https://github.com/krakjoe/apcu/blob/8c05c515b698d5f95555a6afdc00846405d50b40/config.m4#L40-L48

This can probably be closed; I believe that having different processes on Windows with fastcgi have different apcu shared memory caches is expected, and the cache you get will depend on which process it gets sent to. The cache is still useful, because the same process will be used for processing hundreds or thousands or more http requests, depending on the configuration)

I agree that we should not bother "fixing" APCu in this regard, since this is a more general issue (e.g. persistent connections work exactly the same, and even OPcache is, besides that it tries to re-attach on Windows, which is a band-aid, and not a proper solution). But still, the documentation should mention this; since the runtime configuration section already mentions mmap vs. shm, it might be a good place to add some words.

cmb69 commented 1 year ago

This is now documented: https://github.com/php/doc-en/commit/cf3cad79fecf14a50718f490574ada9d3725a4f2.

The ticket can be closed.

nikic commented 1 year ago

Thanks!