php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #71596 Segmentation fault on ZTS with date function (setlocale)
Submitted: 2016-02-15 09:38 UTC Modified: 2016-10-14 20:38 UTC
From: maroszek at gmx dot net Assigned: ab
Status: Closed Package: Date/time related
PHP Version: 7.0.3 OS: Windows
Private report: No CVE-ID:
 [2016-02-15 09:38 UTC] maroszek at gmx dot net
Description:
------------
Description:
------------
PHP 7 is current stable (Also reproducable with 5.6.18)
PHP 7 is used as Embed (Also reproducable with 5.6.18)

Visual Studio 2013

I build a simple example to demonstrate the threading problem. 
It crashes nearly instatly. Running only one thread works fine.

Expected result:
----------------
No crash. Endless loop doing the work. 

Actual result:
--------------
Crash. I attached a backtrace which shows the concurrent access to setlocale which causes the crash



Test script:
---------------
#include <stdio.h>
#include <iostream>
#include <thread>

#include <main/php.h>
#include <main/SAPI.h>
#include <main/php_main.h>
#include <main/php_variables.h>
#include <main/php_ini.h>
#include <zend_ini.h>

#ifdef ZTS
ZEND_TSRMLS_CACHE_EXTERN();
#endif

zend_module_entry ips_module_entry = {
	STANDARD_MODULE_HEADER,
	"XYZ",
	NULL,
	NULL,
	NULL,
	NULL,
	NULL,
	NULL,
	NO_VERSION_YET,
	STANDARD_MODULE_PROPERTIES
};

static size_t php_embed_read_post(char *str, size_t str_length)
{
	return 0;
}

static char* php_embed_read_cookies()
{
	return NULL;
}

static size_t php_embed_ub_write(const char *str, size_t str_length)
{
	std::cout << str;
	return str_length;
}

static void php_embed_flush(void *server_context)
{
	//
}

static void php_embed_send_header(sapi_header_struct *sapi_header, void *server_context)
{
	//
}

static void php_embed_log_message(char *message)
{
	fprintf(stderr, "%s\n", message);
}

static void php_embed_register_variables(zval *track_vars_array)
{
	//
}

static int php_embed_startup(sapi_module_struct *sapi_module)
{
	if (php_module_startup(sapi_module, NULL, 0) == FAILURE) {
		return FAILURE;
	}
	return SUCCESS;
}

sapi_module_struct php_embed_module = {
	"XYZ",                       /* name */
	"PHP for XYZ",               /* pretty name */

	php_embed_startup,                 /* startup */
	php_module_shutdown_wrapper,       /* shutdown */

	NULL,                              /* activate */
	NULL,                              /* deactivate */

	php_embed_ub_write,                /* unbuffered write */
	php_embed_flush,                   /* flush */
	NULL,                              /* get uid */
	NULL,                              /* getenv */

	php_error,                         /* error handler */

	NULL,                              /* header handler */
	NULL,                              /* send headers handler */
	php_embed_send_header,             /* send header handler */

	php_embed_read_post,               /* read POST data */
	php_embed_read_cookies,            /* read Cookies */

	php_embed_register_variables,   /* register server variables */
	php_embed_log_message,          /* Log message */
	NULL,							/* Get request time */
	NULL,							/* Child terminate */

	STANDARD_SAPI_MODULE_PROPERTIES
};

int main(int argc, const char * argv[]) {

	tsrm_startup(128, 1, 0, NULL);
	sapi_startup(&php_embed_module);

	php_embed_module.ini_entries = "date.timezone=Europe/Berlin\n\0";

	if (php_embed_module.startup(&php_embed_module) == FAILURE) {
		throw std::runtime_error("Could not start PHP!");
	}

	for (int i = 0; i < 10; i++) {
		std::thread([&argv]{
			ts_resource(0);

			while (true){
				zend_file_handle file_handle;
				file_handle.type = ZEND_HANDLE_FILENAME;
				file_handle.filename = "date.php";
				file_handle.handle.fp = NULL;
				file_handle.opened_path = NULL;
				file_handle.free_filename = 0;

				if (php_request_startup() == FAILURE) {
					//std::cout << "ERR" << std::endl;
					php_request_shutdown(NULL);
					continue;
				}

				if (php_execute_script(&file_handle) == FAILURE)
					break;

				//std::cout << "RUN" << std::endl;
				php_request_shutdown(NULL);
			}

			std::cout << "DIED" << std::endl;

		}).detach();
	}

	while (true);

	return 0;
}


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-02-15 10:27 UTC] maroszek at gmx dot net
When adding this code before ts_resource(0); the crash remains but looks a little bit different. 
_configthreadlocale(_ENABLE_PER_THREAD_LOCALE);

MSDN: https://msdn.microsoft.com/de-de/library/26c0tb7x.aspx
We a calling this function in SAPI.c in sapi_startup. AFAIK this function is only called for the "main" thread. 
If i read the MSDN right, we need to call this function in all worker threads aswell. Neither one of the SAPIs does this!?
 [2016-02-16 08:20 UTC] ab@php.net
-Status: Open +Status: Feedback
 [2016-02-16 08:20 UTC] ab@php.net
It's great you constantly aim improving the situation with TS builds :)

It looks correct to call configtreadlocale only once in the main thread. That's also what is being done in the snippet from the doc page. However, from the doc page:
[QUOTE]
These functions affect the behavior of setlocale, _tsetlocale, _wsetlocale, _beginthread, and _beginthreadex. If another method is used to create threads, the locale settings have no effect on those threads.
[/QUOTE]

I'm currently unaware whether std::thread() is using _beginthread(), maybe you could trace it? But if it doesn't, setlocale() will be clearly not thread safe so would require locking. 

FYI we use vc11 with PHP5 currently, and vc14 with 7.0. vc12 could possibly have build issues with PHP < 7 and in any case there are no dep libs builds for it.

Cheers.
 [2016-02-16 10:15 UTC] maroszek at gmx dot net
I have another upcoming ZTS bugreport :) and thanks for your time and effort!

std::thread seems to properly use beginthread. Nevertheless i updated the example just to be sure. And i linked to threadlocale.obj to ensure that locales for each thread are enabled. I also compiled the PHP7 example with Visual Studio 2015 Update 1. The error remains :) Can you have a look if you can reproduce it aswell?

Updated Example:
unsigned __stdcall phpThread(void* params)
{
	ts_resource(0);

	while (true) {
		zend_file_handle file_handle;
		file_handle.type = ZEND_HANDLE_FILENAME;
		file_handle.filename = "date.php";
		file_handle.handle.fp = NULL;
		file_handle.opened_path = NULL;
		file_handle.free_filename = 0;

		if (php_request_startup(TSRMLS_C) == FAILURE) {
			//std::cout << "ERR" << std::endl;
			php_request_shutdown(NULL);
			continue;
		}

		if (php_execute_script(&file_handle TSRMLS_CC) == FAILURE)
			break;

		//std::cout << "RUN" << std::endl;
		php_request_shutdown(NULL);
	}

	std::cout << "DIED" << std::endl;

	return 0;
}

int main(int argc, const char * argv[]) {

	tsrm_startup(128, 1, 0, NULL);
	sapi_startup(&php_embed_module);

	php_embed_module.ini_entries = "date.timezone=Europe/Berlin\n\0";

	if (php_embed_module.startup(&php_embed_module) == FAILURE) {
		throw std::runtime_error("Could not start PHP!");
	}

	for (int i = 0; i < 10; i++) {
		_beginthreadex(NULL, 0, phpThread, NULL, 0, NULL);
		//std::thread([]{phpThread(NULL);}).detach();
	}

	while (true);

	return 0;
}
 [2016-02-16 15:15 UTC] ab@php.net
I used your latest variant and have some crash, too. But it's not about set locale

>	php7ts.dll!zend_hash_graceful_reverse_destroy(_zend_array * ht=0x0000020f71c909a0) Line 1489	C
 	php7ts.dll!shutdown_executor() Line 278	C
 	php7ts.dll!zend_deactivate() Line 969	C
 	php7ts.dll!php_request_shutdown(void * dummy=0x0000000000000000) Line 1826	C
 	bug71596.exe!phpThread(void * params=0x0000000000000000) Line 130	C++
 	bug71596.exe!thread_start<unsigned int (__cdecl*)(void * __ptr64)>(void * const parameter=0x00007ff621d579c4) Line 115	C++

It's incomplete yet, have to recompile a debug build to get more info on that. It currently shows a crash while destroying the symbol table on some RSHUTDOWN. This might or might not be related to your locale crash.

Btw, what is the content of your date.php, i was just putting some date("i"); in there from what i saw in your bt.

Thanks.
 [2016-02-16 15:22 UTC] maroszek at gmx dot net
In my first comment i have attached a full project, if that helps.

Contents of date.php:
<?

//ini_set( 'date.timezone', 'Europe/Berlin' );
//date_default_timezone_set( 'Europe/Berlin' );
date("Ymd");
 [2016-02-16 19:15 UTC] ab@php.net
Oh yes, thanks. I was just using the code you posted directly into the thread :) so had nearly the same date() call.

I was finally able to reproduce crashes in setlocale, seems it was only passing by luck previously. I was reading through the docs again, there is another page where the docs are more clear https://msdn.microsoft.com/en-us/library/ms235302.aspx . Looks like what you told in the description is right. This is contradicting another doc page and the code snippet there a bit, telling that the _configthreadlocale has to be called in every thread.

If you apply this patch, rebuild PHP and the test program, do you see any setlocale crashes?

diff --git a/main/SAPI.c b/main/SAPI.c
index 9781f18..4084f7e 100644
--- a/main/SAPI.c
+++ b/main/SAPI.c
@@ -437,6 +437,9 @@ SAPI_API void sapi_activate_headers_only(void)

 SAPI_API void sapi_activate(void)
 {
+#if defined(PHP_WIN32) && defined(ZTS)
+       _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);
+#endif
        zend_llist_init(&SG(sapi_headers).headers, sizeof(sapi_header_struct), (void (*)(void *)) sapi_free_header, 0);
        SG(sapi_headers).send_default_content_type = 1;

With this in, the crash i've posted previously remains. But the setlocale one disappears. You've mentioned previously that adding _configthreadlocale call to every thread changes your backtrace. How does it look like?

Thanks.
 [2016-02-17 10:15 UTC] maroszek at gmx dot net
Thanks! That works flawlessly. 
Tested and works for 5.6.18 aswell, if you want to backport the patch.

Regarding the other issue: I was not able to reproduce the crash using a debug build. I saw it once in the release version... but had no good backtrace. If it happens again with a good backtrace i will post it.

Regarding my statement that the backtrace changed: I think it was only looking a bit different, but was also due to the setlocale bug. It never had to do anything with the shutdown crash.
 [2016-02-17 14:05 UTC] ab@php.net
Thanks very much for the follow up. I'll apply this one to 5.6 then. Bug #71129 is certainly an issue as well, but an unrelated one. Probably it's good so, fixing them step by step and testing good.

Thanks!
 [2016-02-18 18:19 UTC] ab@php.net
-Status: Feedback +Status: Closed -Assigned To: +Assigned To: ab
 [2016-02-18 18:19 UTC] ab@php.net
Applied in 632fc51d98494565a71ae97a81bd7d440dbb116a.

Thanks.
 [2016-10-14 20:38 UTC] rasmus@php.net
-Block user comment: No +Block user comment: Yes
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Wed Jun 28 12:01:42 2017 UTC