Skip to content

Commit

Permalink
Profile: Use grace period to avoid trailing signals from timer (#44268)
Browse files Browse the repository at this point in the history
  • Loading branch information
IanButterworth authored Feb 20, 2022
1 parent 6409a8a commit e723d37
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 10 deletions.
20 changes: 13 additions & 7 deletions src/signals-unix.c
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,16 @@ void usr2_handler(int sig, siginfo_t *info, void *ctx)
errno = errno_save;
}

// Because SIGUSR1 is dual-purpose, and the timer can have trailing signals after being deleted,
// a 2-second grace period is imposed to ignore any trailing timer-created signals so they don't get
// confused for user triggers
uint64_t last_timer_delete_time = 0;

int timer_graceperiod_elapsed(void)
{
return jl_hrtime() > (last_timer_delete_time + 2e9);
}

#if defined(HAVE_TIMER)
// Linux-style
#include <time.h>
Expand Down Expand Up @@ -541,9 +551,7 @@ JL_DLLEXPORT void jl_profile_stop_timer(void)
{
if (running) {
timer_delete(timerprof);
// Because SIGUSR1 is multipurpose, care must be taken for running = 0 to be set after the timer has fully stopped.
// There may be a pending signal emitted from the timer so wait a few timer cycles
sleep_ms((nsecprof / GIGA) * 1000 * 3);
last_timer_delete_time = jl_hrtime();
running = 0;
}
}
Expand Down Expand Up @@ -574,9 +582,7 @@ JL_DLLEXPORT void jl_profile_stop_timer(void)
if (running) {
memset(&timerprof, 0, sizeof(timerprof));
setitimer(ITIMER_PROF, &timerprof, NULL);
// Because SIGUSR1 is multipurpose, care must be taken for running = 0 to be set after the timer has fully stopped.
// There may be a pending signal emitted from the timer so wait a few timer cycles
sleep_ms((nsecprof / GIGA) * 1000 * 3);
last_timer_delete_time = jl_hrtime();
running = 0;
}
}
Expand Down Expand Up @@ -786,7 +792,7 @@ static void *signal_listener(void *arg)
}
#else
if (sig == SIGUSR1) {
if (running != 1)
if (running != 1 && timer_graceperiod_elapsed())
trigger_profile_peek();
doexit = 0;
}
Expand Down
9 changes: 6 additions & 3 deletions stdlib/Profile/test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -200,17 +200,20 @@ if Sys.isbsd() || Sys.islinux()
"""
iob = Base.BufferStream()
p = run(pipeline(`$cmd -e $script`, stderr = devnull, stdout = iob), wait = false)
t = Timer(60) do t # should be done in under 10 seconds
t = Timer(120) do t
# should be under 10 seconds, so give it 2 minutes then report failure
println("KILLING BY PROFILE TEST WATCHDOG\n")
kill(p, Base.SIGTERM)
sleep(10)
kill(p, Base.SIGKILL)
sleep(5)
close(iob)
end
try
s = readuntil(iob, "started", keep = true)
@assert occursin("started", s)
@assert process_running(p)
for _ in 1:2
sleep(2)
sleep(2.5)
if Sys.isbsd()
kill(p, 29) # SIGINFO
elseif Sys.islinux()
Expand Down

0 comments on commit e723d37

Please sign in to comment.