Fix crashes when some external APIs fail #2700
Conversation
The API PyType_Ready may fail, when it happens, the program may crash.
Print error log message when pthread_create fails.
|
@ZhouyangJia Would you mind rebasing the code resolving the conflicts and adjust to individual error messages? I would like to include this PR in the upcoming 5.11.0. |
| PyType_Ready(&ConfigType); | ||
| PyType_Ready(&PluginDataType); | ||
| if (PyType_Ready(&ConfigType) < 0) { | ||
| cpy_log_exception("python initialization"); |
There was a problem hiding this comment.
This and the below error messages are all the same, this makes debugging harder than necessary, please change these to include the actual failure in the error message so each error can be tracked to the respective line.
dago
added
the
Waiting for response - 1st time
|
Hi, I add individual error messages in the error-handling code in #3402. Thanks |
| @@ -71,7 +71,8 @@ static void sig_usr1_handler(int __attribute__((unused)) signal) { | |||
| * so it should be done asynchronously */ | |||
| pthread_attr_init(&attr); | |||
| pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); | |||
| pthread_create(&thread, &attr, do_flush, NULL); | |||
| if (pthread_create(&thread, &attr, do_flush, NULL) != 0) | |||
| fprintf(stderr, "Can not create thread: %s.\n", strerror(errno)); | |||
There was a problem hiding this comment.
At this point we're already forked so stderr is closed.
You're also using strerror() which is not async signal safe.
I don't think handling the error buys us anything here.
| @@ -1257,16 +1257,34 @@ static int cpy_init_python(void) { | |||
| Py_Initialize(); | |||
| python_sigint_handler = PyOS_setsig(SIGINT, cur_sig); | |||
|
|
|||
| PyType_Ready(&ConfigType); | |||
| PyType_Ready(&PluginDataType); | |||
| if (PyType_Ready(&ConfigType) < 0) { | |||
There was a problem hiding this comment.
PyType_Ready returns -1 on failure, please compare it with -1 directly.
Hi,
I analyzed the collectd source code and found some potential API bugs that may cause crashes. These crashes are mainly caused by insufficient error handling of API functions like PyType_Ready or pthread_create.
I think it's unsafe to assume the library function would be correct. It would be better if we could handle the error properly (at least printing a message).
Best,
Zhouyang