Skip to content

Commit

Permalink
Fix the issue of crashes caused by concurrent calls to putenv in a mu…
Browse files Browse the repository at this point in the history
…lti-thread environment.
  • Loading branch information
matyhtf committed Jan 8, 2025
1 parent c8e2a6c commit 3f78da5
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 0 deletions.
68 changes: 68 additions & 0 deletions ext-src/swoole_runtime.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ static PHP_FUNCTION(swoole_time_sleep_until);
static PHP_FUNCTION(swoole_stream_select);
static PHP_FUNCTION(swoole_stream_socket_pair);
static PHP_FUNCTION(swoole_user_func_handler);
#if defined(HAVE_PUTENV) && defined(SW_THREAD)
static PHP_FUNCTION(swoole_putenv);
#endif
#if PHP_VERSION_ID >= 80400
extern PHP_FUNCTION(swoole_exit);
#endif
Expand Down Expand Up @@ -140,6 +143,10 @@ static std::vector<std::string> unsafe_functions {
"pcntl_sigwaitinfo",
};

#if defined(HAVE_PUTENV) && defined(SW_THREAD)
static std::unordered_map<std::string, std::string> swoole_runtime_environ;
#endif

static const zend_function_entry swoole_runtime_methods[] = {
PHP_ME(swoole_runtime, enableCoroutine, arginfo_class_Swoole_Runtime_enableCoroutine, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC)
PHP_ME(swoole_runtime, getHookFlags, arginfo_class_Swoole_Runtime_getHookFlags, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC)
Expand Down Expand Up @@ -289,6 +296,14 @@ void php_swoole_runtime_rinit() {
tmp_function_table = (zend_array *) emalloc(sizeof(zend_array));
zend_hash_init(tmp_function_table, 8, nullptr, nullptr, 0);

#if defined(HAVE_PUTENV) && defined(SW_THREAD)
/**
* There are issues with the implementation of putenv in PHP,
* which can lead to memory invalid read in multi-thread environment.
*/
SW_HOOK_FUNC(putenv);
#endif

if (!sw_is_main_thread()) {
return;
}
Expand Down Expand Up @@ -2324,3 +2339,56 @@ static void clear_class_entries() {
}
child_class_entries.clear();
}

#if defined(HAVE_PUTENV) && defined(SW_THREAD)
/* {{{ Set the value of an environment variable */
static PHP_FUNCTION(swoole_putenv) {
char *setting;
size_t setting_len;
char *p;
bool result;
std::string key;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_STRING(setting, setting_len)
ZEND_PARSE_PARAMETERS_END();

if (setting_len == 0 || setting[0] == '=') {
zend_argument_value_error(1, "must have a valid syntax");
RETURN_THROWS();
}

if ((p = strchr(setting, '='))) {
key = std::string(setting, p - setting);
} else {
key = std::string(setting, setting_len);
}

tsrm_env_lock();
swoole_runtime_environ[key] = std::string(setting, setting_len);
auto iter = swoole_runtime_environ.find(key);

#ifdef HAVE_UNSETENV
if (!p) { /* no '=' means we want to unset it */
unsetenv(iter->second.c_str());
}
if (!p || putenv((char *) iter->second.c_str()) == 0) { /* success */
#else
if (putenv((char *) iter->second.c_str()) == 0) { /* success */
#endif

#ifdef HAVE_TZSET
if (zend_binary_strcasecmp(key.c_str(), key.length(), ZEND_STRL("TZ")) == 0) {
tzset();
}
#endif
result = true;
} else {
result = false;
}

tsrm_env_unlock();
RETURN_BOOL(result);
}
/* }}} */
#endif
6 changes: 6 additions & 0 deletions tests/include/api/swoole_thread/putenv.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php
require __DIR__ . '/../../../include/bootstrap.php';
$args = Swoole\Thread::getArguments();
$val = $args[0] . '_' . time() . '_' . uniqid();
putenv('TEST_THREAD_' . $args[0] . '=' . $val);
Assert::eq(getenv('TEST_THREAD_' . $args[0]), $val);
28 changes: 28 additions & 0 deletions tests/swoole_thread/putenv.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
--TEST--
swoole_thread: putenv
--SKIPIF--
<?php
require __DIR__ . '/../include/skipif.inc';
skip_if_nts();
?>
--FILE--
<?php
require __DIR__ . '/../include/bootstrap.php';

$c = 8;
$threads = [];

for ($i = 0; $i < $c; $i++) {
$threads[] = new Swoole\Thread(TESTS_API_PATH . '/swoole_thread/putenv.php', $i);
}

for ($i = 0; $i < $c; $i++) {
$threads[$i]->join();
}

for ($i = 0; $i < $c; $i++) {
$env = getenv('TEST_THREAD_' . $i);
Assert::notEmpty($env);
}
?>
--EXPECT--

0 comments on commit 3f78da5

Please sign in to comment.