-
Notifications
You must be signed in to change notification settings - Fork 169
fix the hang caused by pthread_cancel #564
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Jing Li <jing.b.li@intel.com>
@lijing0010 you need to put an |
Source/Lib/Codec/EbThreads.c
Outdated
pthread_cancel(*((pthread_t*)threadHandle)); | ||
pthread_join(*((pthread_t*)threadHandle), NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
void *(*start_routine) (void *), void *arg);
On success, pthread_create() returns 0; on error, it returns an error number, and the contents of *thread are undefined.
from: https://man7.org/linux/man-pages/man3/pthread_create.3.html.
Since *thread is undefined, do we even need to call pthread_cancel and pthread_join in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In either case the two levels of return code checking can be condensed (i.e. ret != 0) is not needed.
if (ret == EPERM) is sufficient.
if (ret != 0) {
if (ret == EPERM) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine) (void *), void *arg); On success, pthread_create() returns 0; on error, it returns an error number, and the contents of *thread are undefined.
from: https://man7.org/linux/man-pages/man3/pthread_create.3.html.
Since *thread is undefined, do we even need to call pthread_cancel and pthread_join in this case?
good question:), actually I don't think it's a good idea to turn set the thread policy to real time, it will drain the CPU. Of course it's good for encoding, and can have a little better encoder performance. But in real use case where you have other components like decoders/demuxers etc, doing so will not end up well
Signed-off-by: Jun Tian <jun.tian@intel.com>
Hi @lijing0010 , before we remove setting thread priority code, how about making this clean up first? Could you review? Thank you. |
Co-authored-by: Christopher Degawa <ccom@randomderp.com>
Signed-off-by: Jing Li jing.b.li@intel.com
Description
pthread_cancel is an async call, need to join() before next move, otherwise it may crash.
Issue
If init/deinit for about 150 times with default input parameter, will crash at set_thread_affinity()
Author(s)
Performance impact
Merge method