Bug #71596 [Fbk]: Segmentation fault on ZTS with date function (setlocale)

From: Date: Tue, 16 Feb 2016 19:15:35 +0000
Subject: Bug #71596 [Fbk]: Segmentation fault on ZTS with date function (setlocale)
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-199273@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=71596&edit=1

 ID:                 71596
 Updated by:         ab@php.net
 Reported by:        maroszek at gmx dot net
 Summary:            Segmentation fault on ZTS with date function
                     (setlocale)
 Status:             Feedback
 Type:               Bug
 Package:            Date/time related
 Operating System:   Windows
 PHP Version:        7.0.3
 Block user comment: N
 Private report:     N

 New Comment:

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.


Previous Comments:
------------------------------------------------------------------------
[2016-02-16 15:22:46] 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 15:15:29] 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 10:15:10] 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 08:20:07] 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-15 10:27:02] 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!?

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=71596


--
Edit this bug report at https://bugs.php.net/bug.php?id=71596&edit=1


Thread (36 messages)

« previous php.bugs (#199273) next »