Skip to content

tests/server: make the signal handler signal-safe #16852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 14 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Mar 28, 2025

Before this patch the signal handler called logmsg() which in turn
called printf() variants (internal implementations), and FILE *
functions, localtime(). Some of these called malloc/free, which
isn't supported in s signal handler. Replace them with write calls,
losing some logging functionality.

Also:

POSIX specs with list of functions allowed in a signal handler:
2004: https://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html#tag_02_04_03
2017: https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03
2024: https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html#tag_16_04_03

Linux CI run with the thread sanitizer going crazy when
hitting the signal handler in test 1238 and 1242 (TFTP):

WARNING: ThreadSanitizer: signal-unsafe call inside of a signal (pid=12582)
    #0 malloc <null> (servers+0x5ed70)
    #1 _IO_file_doallocate <null> (libc.so.6+0x851b4)
    #2 formatf /home/runner/work/curl/curl/bld/tests/server/../../lib/../../lib/mprintf.c:886:9 (servers+0xdff77)
[...]
WARNING: ThreadSanitizer: signal-unsafe call inside of a signal (pid=12582)
    #0 free <null> (servers+0x5f453)
    #1 fclose <null> (libc.so.6+0x8532f)
    #2 logmsg /home/runner/work/curl/curl/bld/tests/server/../../../tests/server/util.c:134:5 (servers+0xe684d)

Ref: https://github.com/curl/curl/actions/runs/14118903372/job/39555309490?pr=16851

@vszakats vszakats added the tests label Mar 28, 2025
This caused no error, but it's incorrect and introduced a potential
difference to builds without `STDERR_FILENO`, which in practice
means MSVC. The macro's only use needs to use `stderr` always.

Follow-up to e5bb88b curl#11958
@vszakats vszakats changed the title tests/server: be signal safe in the signal handler tests/server: make the signal handler signal-safe Mar 28, 2025
@vszakats vszakats closed this in e95f509 Mar 28, 2025
@vszakats vszakats deleted the tsan branch March 28, 2025 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants