Skip to content

Conversation

matyhtf
Copy link
Contributor

@matyhtf matyhtf commented Oct 22, 2024

In the Swoole extension, including zend_max_execution_timer.h and calling the zend_max_execution_timer_init function leads to an undefined symbol error. This function should be exported in C symbol format.

php: symbol lookup error: swoole.so: undefined symbol: _Z29zend_max_execution_timer_initv

BEGIN_EXTERN_C()
#include "zend_long.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the #include outside of the BEGIN_EXTERN_C() block

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

matyhtf referenced this pull request in swoole/swoole-src Oct 22, 2024
@matyhtf
Copy link
Contributor Author

matyhtf commented Oct 22, 2024

The solution to avoid modifying this file is to add extern "C" around #include php.ini.

#ifdef __cplusplus
extern "C" {
#endif

#include "php.h"

#ifdef __cplusplus
}
#endif

@matyhtf
Copy link
Contributor Author

matyhtf commented Oct 23, 2024

ad85e71

It is advisable not to add new header files in the release version. The file zend_max_execution_timer.h was added in PHP version 8.1.18RC1, whereas the PHP version in the Ubuntu 22.04 apt repository is 8.1.3. Consequently, using the zend_max_execution_timer API will result in a compilation failure on Ubuntu systems.

@dunglas

@dunglas
Copy link
Member

dunglas commented Oct 23, 2024

@matyhtf Yes we know that but this is both a bug fix and a feature. Without this new system, timeouts are totally broken in multithreaded programs and there is nothing else we can do to fix that. This feature is off by default for PHP 8.1 if not compiled in ZTS mode, so the ABI didn't change for most users.


BEGIN_EXTERN_C()
/* Must be called after calls to fork() */
ZEND_API void zend_max_execution_timer_init(void);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only this function must be called in extensions if they fork(). It nay not be necessary to protect the two other definitions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, updated.

@arnaud-lb arnaud-lb merged commit 4a4371d into php:master Oct 24, 2024
9 of 10 checks passed
@arnaud-lb
Copy link
Member

Thank you @matyhtf!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants