Sending a notification to the main nginx thread

[Eran Kornblau]

> 
> -----Original Message-----
> From: nginx-devel <nginx-devel-bounces at nginx.org> On Behalf Of Maxim Dounin
> Sent: Thursday, 7 October 2021 20:17
> To: nginx-devel at nginx.org
> Subject: Re: Sending a notification to the main nginx thread
> 
> 
> Hello!
> 
> In no particular order:
> 
> - Assuming nginx uses only POSIX functions is wrong, it does use
>   platform-specific functions and various portable functions not
>   specified by POSIX, such as
> 
> - The above objdump results look incorrect: for example, nginx
>   certainly uses readdir(), which is in the POSIX non-thread-safe
>   list, but not in your list.
> 
> - There are other libraries nginx uses, which makes the problem
>   much worse.
> 
> If you insist on using threads in your module - you are free to do so, you were warned and it's your choice.
> 

Thanks Maxim, did some more research following your feedback, just for my understanding...
I'm sharing the results here in case someone will find it useful.

First of all, regarding readdir, you are right, of course.
The reason it was missing is that it shows as readdir64 in the import table,
while the list of non-thread-safe POSIX functions lists only readdir.

I ran a looser check - grepped the unsafe POSIX functions, but without assuming an exact match -

objdump -T /usr/local/nginx/sbin/nginx | grep -v ngx_ | grep -e asctime -e basename ... -e wctomb
0000000000000000      DF *UND*  0000000000000000  GLIBC_2.2.5 getenv
0000000000000000      DF *UND*  0000000000000000  GLIBC_2.2.5 localtime
0000000000000000      DF *UND*  0000000000000000  GLIBC_2.2.5 getpwnam
0000000000000000      DF *UND*  0000000000000000  GLIBC_2.2.5 getgrnam
0000000000000000      DF *UND*  0000000000000000  GLIBC_2.2.5 readdir64
0000000000000000      DF *UND*  0000000000000000  GLIBC_2.2.5 strerror
0000000000000000      DF *UND*  0000000000000000  GLIBC_2.2.5 dlerror

0000000000000000      DF *UND*  0000000000000000  GLIBC_2.2.5 srandom
0000000000000000      DF *UND*  0000000000000000  GLIBC_2.2.5 random
0000000000000000      DF *UND*  0000000000000000  GLIBC_2.2.5 localtime_r
0000000000000000      DF *UND*  0000000000000000  GLIBC_2.2.5 crypt_r
0000000000000000      DF *UND*  0000000000000000  GLIBC_2.2.5 gmtime_r

Other than readdir64, this added only a few false positives.

Regarding external dependencies, I looked at the basic deps used by nginx (didn't check more esoteric deps, such as libxslt etc.)
zlib & pcre claim to be thread-safe, openssl seems more problematic...
Didn't dig too deep, but seems that at least on older versions, you have to explicitly provide some callbacks to make it thread safe.

I then proceeded to pull a list of all POSIX functions -
(for i in {a..z}; do curl -s https://pubs.opengroup.org/onlinepubs/9699919799/idx/i$i.html | grep '()' | awk '-F(' '{print $1}' | awk '-F>' '{print $NF}'; done) > /tmp/posix-list

Compiled nginx with some basic settings -
auto/configure --with-file-aio --with-http_ssl_module --with-http_v2_module --with-stream --with-debug --with-threads --with-cc-opt="-O0" ; make install

And checked the imports while excluding POSIX, zlib, pcre & openssl -

(objdump -T /usr/local/nginx/sbin/nginx | grep UND | grep -v OPENSSL | grep -v pcre_ | grep -v deflate | awk '{print $NF}' ; cat /tmp/posix-list /tmp/posix-list) | sort | uniq -c | grep -w 1

      1 accept4
      1 __cmsg_nxthdr
      1 crypt_r
      1 epoll_create
      1 epoll_ctl
      1 epoll_wait
      1 __errno_location
      1 eventfd
      1 ftruncate64
      1 __fxstat64
      1 __fxstatat64
      1 getpagesize
      1 getrlimit64
      1 glob64
      1 globfree64
      1 __gmon_start__
      1 initgroups
      1 _ITM_deregisterTMCloneTable
      1 _ITM_registerTMCloneTable
      1 _Jv_RegisterClasses
      1 __libc_start_main
      1 __lxstat64
      1 mmap64
      1 open64
      1 openat64
      1 posix_fadvise64
      1 prctl
      1 pread64
      1 pwrite64
      1 pwritev64
      1 readdir64
      1 sched_setaffinity
      1 sendfile64
      1 setrlimit64
      1 __stack_chk_fail
      1 statfs64
      1 syscall
      1 usleep
      1 __xstat64

Other than readdir64, which was already discussed, I don't spot any non-thread-safe functions, but maybe I'm missing something
(have to admit, I'm not familiar with some of the weird ones here...)

Bottom line, if we put aside openssl which is a bit unclear (I don't use it in my use case, so don't care much ;-)),
the problematic library functions used by nginx (at least on Linux...) seem to be - strerror, localtime, readdir.

Even though I agree with the general statement that thread use in nginx should be discouraged, sometimes it's the lesser evil...
IMHO, would be better to update nginx to use the thread-safe / _r variants on platforms that support them.
I don't see any downside to it, other than a couple more #if's in the code...
The world would have been better if such unsafe functions would have never been born :)

Thank you for your time!

Eran

> --
> Maxim Dounin
> https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmdounin.ru%2F&data=04%7C01%7C%7C60f75b5309df4989493108d989b64f04%7C0c503748de3f4e2597e26819d53a42b6%7C1%7C0%7C637692238383728263%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=2g%2FXvh1VsIgO%2F3jBOYQF%2Ba%2Fo3QfQFKUNcjZ5jCN%2Fke8%3D&reserved=0
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmailman.nginx.org%2Fmailman%2Flistinfo%2Fnginx-devel&data=04%7C01%7C%7C60f75b5309df4989493108d989b64f04%7C0c503748de3f4e2597e26819d53a42b6%7C1%7C0%7C637692238383728263%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=znw%2FdNVtzw6gd42ceMMeUgM20JbGvQClpksJYidQx8M%3D&reserved=0
>