dankamongmen / notcurses

blingful character graphics/TUI library. definitely not curses.
https://nick-black.com/dankwiki/index.php/Notcurses
Other
3.48k stars 112 forks source link

pthread_cancel() is bad. #2675

Open bilrou opened 2 years ago

bilrou commented 2 years ago

The presence of pthread_cancel() hinders portability on arm-based platforms. We need to look for less problematic alternatives to pthread_cancel(). Look
https://github.com/termux/termux-packages/issues/8860 https://github.com/termux/termux-packages/pull/11626

dankamongmen commented 2 years ago

i didn't see anything in those two links about arm portability problems due to pthread_cancel(), and if there were, i'd honestly be interested in resolving them in the appropriate libcs. Notcurses is run through armhf and arm64 test suites on both the Debian and Fedora environments as part of package building there, and I use it on several ARM platforms locally -- can you provide more details as to the portability concerns? pthread_cencel() is part of POSIX, and widely uised.

bilrou commented 2 years ago

In termux we have the bionic handle which is a little different from the standard libc. A patch could solve the problem... but it's hard to get one that gives stability and security for this particular function.

/home/builder/.termux-build/notcurses/src/src/lib/internal.h:1836:6: warning: implicit declaration of function 'pthread_cancel' is invalid in C99 [-Wimplicit-function-declaration]
  if(pthread_cancel(tid)){
     ^   
dankamongmen commented 2 years ago

ahhh, ok, confirmed that pthread_cancel() sounds like it might have problems on Android Bionic.

let me ponder how intensely we make use of pthread_cancel(). it might be easily replaced, but i doubt it. i'm not going to be interested in major surgery around carefully-reasoned thread cancellation, but maybe there's an easy alternative path...

phanirithvij commented 12 months ago

The package has been patched to handle pthread_cancel and other things and is available in termux-packages https://github.com/termux/termux-packages/pull/13125