Don't involve PHP in Apache mpm_winnt control process

[cmb69]

Apache mpm_winnt uses a single control process which launches a single
child process which in turn creates threads to handle requests. Since
the child process is not forked, but rather spawned, and the control
process is never involved in request handling, there is no need to
power up PHP for the control process. Besides not doing this saves
some resources, we no longer have to deal with potential re-attaching
to OPcache shared memory which avoids issues due to ASLR, and paves the
way to further improvements (such as preloading support for Apache
mpm_winnt).

---

Possibly, a problem with that exact implementation is that it assumes that mpm_winnt is always used on Windows, what might be wrong. Maybe someone knowlegable with Apache on Windows can clarify whether alternative MPMs might be used there, and if so, suggest a better way to detect that (i.e. is there a respective macro we could use in favor of PHP_WIN32)?

cc @Jan-E

[Jan-E]

Hi Christoph,
In my case mpm_winnt is always used, either on my dev laptop, on a staging server or in production.
I have asked it on Apachelounge:
https://www.apachelounge.com/viewtopic.php?t=8818
Maybe there are odd configs out there with an alternative MPM.

[Jan-E]

@wrowe Are you aware of Apache setups on Windows with an alternative MPM ?

[cmb69]

Hmm, just seen

php-src/ext/opcache/shared_alloc_win32.c

Lines 221 to 223 in 1d2c544

	/* Mapping retries: When Apache2 restarts, the parent process startup routine
	can be called before the child process is killed. In this case, the mapping will fail
	and we have to sleep some time (until the child releases the mapping object) and retry.*/

So running PHP for the control job might be deliberate: if a worker thread crashes, the whole worker process crashes, but OPcache is still up, and can be re-attached to (at least on good days).

[arnaud-lb]

If this is the only reason, running PHP in the control job may not be worth it. Crashing may leave SHM in a corrupted state (because an SHM update was interrupted), so it's risky to re-use it after.

[cmb69]

Crashing may leave SHM in a corrupted state (because an SHM update was interrupted), so it's risky to re-use it after.

Oh, very good point; I haven't thought about that! However, we have basically the same problem with other SAPIs, too. If e.g. an FPM worker crashes, SHM might be corrupt, but would still be reused, if I'm not wrong. Invalidating SHM for any crash which would not affect OPcache, could be a serious issue for large applications under high traffic.

[arnaud-lb]

I think you are right about FPM. However in a threaded SAPI a crash kills all requests, which is far more disastrous. The cost of re-compiling due to SHM invalidation would probably be minor compared to that.

Also, under Windows, the existence of opcache.file_cache_fallback implies that re-attaching may fail (because the memory region is not available due to ASLR?). When this happens, the impact on performance will be far worse than invalidating SHM, since the process will be slow forever. If we didn't try to re-attach at all, and simply created a new SHM, we wouldn't need opcache.file_cache_fallback.

[cmb69]

Your reasoning makes perfect sense, @arnaud-lb. Thanks!

re-attaching may fail (because the memory region is not available due to ASLR?)

Indeed. We try to map to a certain base address, and if that fails (can only be caused by ASLR), we fall back to file cache (or bail out if not configured).

Anyhow, I've rebased onto master, to resolve the merge conflict, and also adjusted the comment in shared_alloc_win32.c (I'm not sure if that waiting is still required for other servers/SAPIs). I've also did some testing against Symfony demo, and couldn't find any issues.

[arnaud-lb]

I wasn't aware of how re-attachment happened precisely and I just took a look at create_segment() now. My understanding is that calling OpenFileMapping(flags, name) in different processes will end up opening a mapping to the same file if name is the same? Or is there something that's inherited by spawns that lets them see the same file? In the former case we may need more changes to prevent re-attachment.

We attempt to map the opened file to fixed addresses (vista_mapping_base_set) with MapViewOfFileEx(), when creating the initial mapping. Can this fail due to ASLR?

In zend_shared_alloc_reattach(), we check if the address of execute_ex changed. If it did, a fatal error is triggered:

php-src/ext/opcache/shared_alloc_win32.c

Line 168 in 3b23de3

zend_win_error_message(ACCEL_LOG_FATAL, "Opcode handlers are unusable due to ASLR. Please setup opcache.file_cache and opcache.file_cache_fallback directives for more convenient Opcache usage", err);

It is my understanding that unless file_cache_fallback is enabled, this will always fail when ASLR is enabled, so re-attachment always fallbacks to the file cache.

As we are storing addresses in cached op arrays now, re-attachment is never safe with ASLR.

I think we may as well rewrite create_segments() to remove re-attachment support, and to use VirtualAlloc() :)

[arnaud-lb]

Unfortunately this accesses PG(expose_php), which is controlled by ini settings. It's too early to call this as ini settings haven't been processed yet. It may be safe to move into php_apache_startup_php() since it's called once per process.

[cmb69]

As is, that is already moved into php_apache_child_init() for PHP_WIN32, and apparently works (tested with default, and both alternatives in php.ini).

[cmb69]

My understanding is that calling OpenFileMapping(flags, name) in different processes will end up opening a mapping to the same file if name is the same?

Right, only if the name (username + zend_system_id + sapi_name) matches, re-attachment is attempted.

We attempt to map the opened file to fixed addresses (vista_mapping_base_set) with MapViewOfFileEx(), when creating the initial mapping. Can this fail due to ASLR?

If none of the attempted addresses (hardcoded vista_mapping_base_set + opcache.mmap_base INI setting) is available, mapping fails (rather unlikely in practice). Re-attaching will try to use the same base address, and that might fail, but most of the time it does not (assuming 64bit). Of course that depends on the chosen mapping address, how many DLLs are involved, and how Windows handles ASLR – the point is that the 64bit address space is so large, that it's somewhat unlikely that the OS reserves the SHM base address for some other purpose. And if all involved binaries have been built with /HIGHENTROPYVA:NO, a high base address should always be free.

As we are storing addresses in cached op arrays now, re-attachment is never safe with ASLR.

Do we really store direct addresses, except for preloading and some other places which are disabled for Windows already?

I think we may as well rewrite create_segments() to remove re-attachment support, and to use VirtualAlloc() :)

Well, I'm not sure whether there is a performance difference when using VirtualAlloc vs. memory mapping (except for startup), and it might already be possible to specify a different opcache.cache_id for each worker process. Some way or another it should be possible to have a separate SHM for each worker process already, what might be a viable alternative for those where ASLR otherwise keeps them from using OPcache SHM at all (or for some workers at least). However, I guess in many use-cases the current re-attachment "works", and you can build your own PHP with /DYNAMICBASE:NO (or use editbin.exe on pre-built binaries) so ASLR is effectively disabled. And I think it is still possible to configure Windows so that this works (if it's not even still the default).

Thinking about this, we could actually build with /DYNAMICBASE:NO, and tell users to allow that on their machines (or possibly detect that at runtime, and fall back to file cache otherwise; users could trade additional security for speed), but that is not supported on ARM64 (and sooner or later we have to deal with that platform).

That said, I still like to get rid of the re-attaching, but in my opinion, we need to provide solid alternatives to FCGI first.

[arnaud-lb]

As we are storing addresses in cached op arrays now, re-attachment is never safe with ASLR.

Do we really store direct addresses, except for preloading and some other places which are disabled for Windows already?

We normally don't in win32 builds, but I realized earlier today after reading the error message, that we store the absolute op code handler address in zend_op->handler. We also store absolute jump addresses and literal addresses in znode_op on x86. We may have introduced other cases of this without being noticed, since re-attaching is effectively disabled on ASLR now.

I think we may as well rewrite create_segments() to remove re-attachment support, and to use VirtualAlloc() :)

Well, I'm not sure whether there is a performance difference when using VirtualAlloc vs. memory mapping (except for startup), and it might already be possible to specify a different opcache.cache_id for each worker process. Some way or another it should be possible to have a separate SHM for each worker process already, what might be a viable alternative for those where ASLR otherwise keeps them from using OPcache SHM at all (or for some workers at least). However, I guess in many use-cases the current re-attachment "works", and you can build your own PHP with /DYNAMICBASE:NO (or use editbin.exe on pre-built binaries) so ASLR is effectively disabled. And I think it is still possible to configure Windows so that this works (if it's not even still the default).

Thinking about this, we could actually build with /DYNAMICBASE:NO, and tell users to allow that on their machines (or possibly detect that at runtime, and fall back to file cache otherwise; users could trade additional security for speed), but that is not supported on ARM64 (and sooner or later we have to deal with that platform).

Ok I was suggesting VirtualAlloc() mostly for simplicity, in case we want to remove re-attaching support completely.

I think that we should not build with /DYNAMICBASE:NO by default, or recommend it, because this would result in less secure binaries. Especially if it's only to enable SHM re-attach, IMHO it's not worth it.

That said, I still like to get rid of the re-attaching, but in my opinion, we need to provide solid alternatives to FCGI first.

What do we have currently? FCGI is php-cgi in FastCGI mode, or FPM ? (I don't know if fpm is supported on Windows). Both would indeed benefit from re-attachment (although it probably always fallback to file currently).

Besides FCGI, we do have the Apache2 module.

And #17838 in the future.

Keeping re-attachment support around until #17838 is released sounds good to me.

Maybe we can disable it explicitly in the Apache2 module?

[cmb69]

What do we have currently? FCGI is php-cgi in FastCGI mode, or FPM ? (I don't know if fpm is supported on Windows).

FPM is not supported on Windows (there is no fork(2)). So yes, FCGI is php-cgi in FastCGI mode.

To be able to do some testing, I've applied the following

patch

 ext/opcache/ZendAccelerator.h         |  1 +
 ext/opcache/shared_alloc_win32.c      | 16 +++++++++++++---
 ext/opcache/zend_accelerator_module.c |  4 +++-
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h
index 440d09e235d..6b1d5a01b8d 100644
--- a/ext/opcache/ZendAccelerator.h
+++ b/ext/opcache/ZendAccelerator.h
@@ -192,6 +192,7 @@ typedef struct _zend_accel_directives {
 #endif
 #ifdef ZEND_WIN32
 	char *cache_id;
+	bool per_worker;
 #endif
 } zend_accel_directives;
 
diff --git a/ext/opcache/shared_alloc_win32.c b/ext/opcache/shared_alloc_win32.c
index 3f0c71a7c66..ac311b8bf6c 100644
--- a/ext/opcache/shared_alloc_win32.c
+++ b/ext/opcache/shared_alloc_win32.c
@@ -266,8 +266,13 @@ static int create_segments(size_t requested_size, zend_shared_segment ***shared_
 	shared_segment = (zend_shared_segment *)((char *)(*shared_segments_p) + sizeof(void *));
 	(*shared_segments_p)[0] = shared_segment;
 
-	memfile	= CreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_EXECUTE_READWRITE | SEC_COMMIT, size_high, size_low,
-								create_name_with_username(ACCEL_FILEMAP_NAME));
+	if (ZCG(accel_directives).per_worker) {
+		memfile	= CreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_EXECUTE_READWRITE | SEC_COMMIT, size_high, size_low, NULL);
+	} else {
+		memfile	= CreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_EXECUTE_READWRITE | SEC_COMMIT, size_high, size_low,
+									create_name_with_username(ACCEL_FILEMAP_NAME));
+	}
+	zend_accel_error(ACCEL_LOG_WARNING, "CreateFileMapping %p", memfile);
 	if (memfile == NULL) {
 		err = GetLastError();
 		zend_shared_alloc_unlock_win32();
@@ -296,7 +301,12 @@ static int create_segments(size_t requested_size, zend_shared_segment ***shared_
 	}
 
 	do {
-		shared_segment->p = mapping_base = MapViewOfFileEx(memfile, FILE_MAP_ALL_ACCESS|FILE_MAP_EXECUTE, 0, 0, 0, *wanted_mapping_base);
+		if (ZCG(accel_directives).per_worker) {
+			shared_segment->p = mapping_base = MapViewOfFileEx(memfile, FILE_MAP_ALL_ACCESS|FILE_MAP_EXECUTE, 0, 0, 0, NULL);
+		} else {
+			shared_segment->p = mapping_base = MapViewOfFileEx(memfile, FILE_MAP_ALL_ACCESS|FILE_MAP_EXECUTE, 0, 0, 0, *wanted_mapping_base);
+		}
+		zend_accel_error(ACCEL_LOG_WARNING, "MapViewOfFileEx at %p", mapping_base);
 		if (*wanted_mapping_base == NULL) { /* Auto address (NULL) is the last option on the array */
 			break;
 		}
diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c
index e6642c76e1a..8cab0f125fd 100644
--- a/ext/opcache/zend_accelerator_module.c
+++ b/ext/opcache/zend_accelerator_module.c
@@ -313,7 +313,8 @@ ZEND_INI_BEGIN()
 #endif
 #ifdef ZEND_WIN32
 	STD_PHP_INI_ENTRY("opcache.cache_id"                      , ""    , PHP_INI_SYSTEM, OnUpdateString,           accel_directives.cache_id,               zend_accel_globals, accel_globals)
-#endif
+	STD_PHP_INI_BOOLEAN("opcache.per_worker"                  , "0"   , PHP_INI_SYSTEM, OnUpdateBool,             accel_directives.per_worker,             zend_accel_globals, accel_globals)
+	#endif
 #ifdef HAVE_JIT
 	STD_PHP_INI_ENTRY("opcache.jit"                           , "disable",                    PHP_INI_ALL,    OnUpdateJit,      options,               zend_jit_globals, jit_globals)
 	STD_PHP_INI_ENTRY("opcache.jit_buffer_size"               , ZEND_JIT_DEFAULT_BUFFER_SIZE, PHP_INI_SYSTEM, OnUpdateLong,     buffer_size,           zend_jit_globals, jit_globals)
@@ -829,6 +830,7 @@ ZEND_FUNCTION(opcache_get_configuration)
 #endif
 #ifdef ZEND_WIN32
 	add_assoc_string(&directives, "opcache.cache_id", STRING_NOT_NULL(ZCG(accel_directives).cache_id));
+	add_assoc_bool(&directives, "opcache.per_worker", ZCG(accel_directives).per_worker);
 #endif
 #ifdef HAVE_JIT
 	add_assoc_string(&directives, "opcache.jit", JIT_G(options));

Then I did a simple comparison against a single Symfony demo URL (de/blog/). First I triggered a single request to that URL, what always spins up a single FCGI worker with usual IIS configuration. Then I've triggered concurrent request against that same URL (spawned 2 additional workers). With per_worker=0 (same as what we have now), the longest request was half of the time than with per_worker=1. And that only for the rather simple Symfony demo for a single URL. For complex applications, when spawning a new worker it likely takes much more time until its OPcache SHM would be warmed up.

Further issues with a SHM per worker:

• OPcache stats would be pretty meaningless for the application
• invalidating/resetting would always only ever work per worker
• more memory consumption

I'm afraid that having a separate OPcache SHM per process is only viable if there is only a single worker process (with multiple worker threads), and even then it might be necessary to keep the worker process up, even if a thread crashes, like it's currently done for ISAPI; need to do some experiments with that).

[arnaud-lb]

I agree with your conclusion that having a separate SHM per process is only viable if there is only one process. My understanding is it will be the case for Apache2 and ISAPI.

One thing to take into consideration when measuring the effect of losing SHM after a crash is that if reattachment was removed entirely, win32 would benefit from the same level of optimizations as other platforms. Normal execution would be faster, and would offset the increased cost of occasional crashes.