Re: [libmicrohttpd] stop_daemon is not thread-safe in 0.9.22 with MHD_USE_THREAD_PER_CONNECTION

[Christian Grothoff]

Dear Dilyan,

Not every race is a bug.  In all of the cases you mention, the code 
*expects* "undefined" results (as in, a race where either thread may 
win) and is written to work fine in either case (to the very best of my 
knowledge --- the races you mention were all known when this part was 
designed!).

Furthermore, putting a mutex around the code accessing these variables 
would cause a major loss of performance (possibly even defeating our 
strategy for load balancing in the thread-pool model, which is to race 
for the listen socket!).  So there is a reason why the code is written 
the way it is.  Valgrind allows you to add suppression rules, so this is 
likely the best way to avoid seeing these false-positives.

Happy hacking!

Christian

On 10/31/2012 04:04 PM, address@hidden wrote:
> Hello,
>
> I start MHD with MHD_USE_THREAD_PER_CONNECTION . version 0.9.22, so name
> libmicrohttpd.so.10.16.1, compiled with CFLAGS="-fno-strict-aliasing -O0
> -ggdb3" ./configure  --disable-curl --disable-https .
>
> According to the messages below, either libmicrohttpd is not thread-safe,
> or I use the API is a wrong way.
>   
>
> When MHD_stop_daemon is called, valgrind+helgind 3.8.1 report:
>
> ---Thread-Announcement------------------------------------------
>
> Thread #8 was created
>     at 0x5F1AC7E: clone (clone.S:77)
>     by 0x53F2F6F: do_clone.constprop.4 (createthread.c:75)
>     by 0x53F457F: pthread_create@@GLIBC_2.2.5 (createthread.c:256)
>     by 0x4C2F777: pthread_create_WRK (in
> /usr/local/lib/valgrind/vgpreload_helgri
>     by 0x51DF44F: create_thread (daemon.c:816)
>     by 0x51E21B0: MHD_start_daemon_va (daemon.c:2414)
>     by 0x51E0D58: MHD_start_daemon (daemon.c:1675)
>     by 0x46530C: main (xyz.cpp:457)
>
> ----------------------------------------------------------------
>
> == first issue ==
>
> Possible data race during write of size 4 at 0x6884174 by thread #1
> Locks held: none
>     at 0x51E276E: MHD_stop_daemon (daemon.c:2623)
>
> This conflicts with a previous read of size 4 by thread #8
> Locks held: none
>     at 0x51E0C87: MHD_select_thread (daemon.c:1641)
>
> Address 0x6884174 is 276 bytes inside a block of size 368 alloc'd
>     at 0x4C2BF7B: malloc (in
> /usr/local/lib/valgrind/vgpreload_helgrind-amd64-lin
>     by 0x51E19A7: MHD_start_daemon_va (daemon.c:2080)
>     by 0x51E0D58: MHD_start_daemon (daemon.c:1675)
>
> meaning, that two threads (the main thread 1 and the thread 8, created my
> MHD as described above) access daemon->shutdown, without synchronizing
> each other, one of the thread writes, the other reads.  This leads to
> undefined results.
>
> Could you please insert some mutex, that ensures that no two threads
> access at the same time daemon->shutdown ?
>
> == second issue==
>
> Another warning is:
>
> Possible data race during write of size 4 at 0x6884168 by thread #1
> Locks held: none
>     at 0x51E2789: MHD_stop_daemon (daemon.c:2625)
>
> This conflicts with a previous read of size 4 by thread #8
> Locks held: none
>     at 0x51E0158: MHD_select (daemon.c:1340)
>     by 0x51E0C63: MHD_select_thread (daemon.c:1644)
>     by 0x4C2C36D: mythread_wrapper (in
> /usr/local/lib/valgrind/vgpreload_helgrind
>     by 0x53F3E99: start_thread (pthread_create.c:308)
>     by 0x5F1ACBC: clone (clone.S:112)
>
> Address 0x6884168 is 264 bytes inside a block of size 368 alloc'd
>     at 0x4C2BF7B: malloc (in
> /usr/local/lib/valgrind/vgpreload_helgrind-amd64-lin
>     by 0x51E19A7: MHD_start_daemon_va (daemon.c:2080)
>     by 0x51E0D58: MHD_start_daemon (daemon.c:1675)
>
> meaning, that two threads access
>    daemon->socket_fd
> one read, one write, without synchronizing each other.  Could you please
> guard the access to daemon->socket_fd with some mutex?
>
> == third and issue ==
> is the same as the first issue, but here
>     MHD_select (daemon.c:1371), called from
>     MHD_select_thread (daemon.c:1644)
> and
>     MHD_stop_daemon (daemon.c:2623)
> make concurrent access.
>
> == fourth issue ==
> is the same as the first issue, but here
>     MHD_select_thread (daemon.c:1641)
> and
>     MHD_stop_daemon (daemon.c:2623)
> do concurrent access without synchronisation.
>
> Shall I make use of the API is a special way, so that the above warnings
> do not come,or does valgrind+helgrind 3.8.1 function suboptimal, or would
> it be possible to fix the issues in libmicrohttpd?
>
> I compile valgrind 3.8.1 with CFLAGS="-Wno-long-long -pipe -O3 -Wl,-O1
> -Wl,-S -Wl,--hash-style=gnu -march=native  -Wno-pointer-sign
> -fno-stack-protector" ./configure --enable-tls , libc is 2.15
>
> Thanks in advance for your attention
>    Dilyan Palauzov
>
>
> ___________________________________________________________________________________________________
>
> Diehl AKO Stiftung & Co. KG, Pfannerstraße 75, 88239 Wangen im Allgäu
> Bereichsvorstand: Dipl.-Ing. (FH) Dieter Neugebauer (Sprecher), Dr. rer. pol. 
> Christoph Weigand
> Sitz der Gesellschaft: Wangen i.A. – Registergericht: Amtsgericht Ulm HRA 
> 620609  – Persönlich haftende Gesellschafterin: Diehl Verwaltungs-Stiftung – 
> Sitz: Nürnberg – Registergericht: Amtsgericht Nürnberg HRA 11756 –  Vorstand: 
> Dr.-Ing. E.h. Thomas Diehl (Vorsitzender), Herr Dipl.-Wirtsch.-Ing. Wolfgang 
> Weggen (stellvertretender Vorsitzender), Dipl.-Kfm. Claus Günther, Dipl.-Ing. 
> (FH) Dieter Neugebauer, Prof. Dr.-Ing. Heiner Otten, Dipl.-Betriebsw. (FH) 
> Johannes Sappa, Dipl.-Kfm. Dr.-Ing. Martin Sommer, Dipl.-Ing. (FH) Rainer von 
> Borstel,  Vorsitzender des Aufsichtsrates: Dipl.-Kfm. Claus Mänz-Siebje
> ___________________________________________________________________________________________________
>
> Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail 
> enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
> Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten 
> haben. Bitte loeschen Sie in diesem Fall die Nachricht. Jede unerlaubte Form 
> der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation 
> dieser E-Mail ist strengstens untersagt.
>
> The contents of the above mentioned e-mail is not legally binding. This e-mail 
> contains confidential and/or legally protected information. Please inform us if 
> you have received this e-mail by mistake and delete it in such a case. Each 
> unauthorized reproduction, disclosure, alteration, distribution and/or 
> publication of this e-mail is strictly prohibited.