xz: Add SIGTSTP handler for progress indicator time keeping.

This way, if xz is stopped the elapsed time and estimated time
remaining won't get confused by the amount of time spent in
the stopped state.

This raises SIGSTOP. It's not clear to me if this is the correct way.
POSIX and glibc docs say that SIGTSTP shouldn't stop the process if
it is orphaned but this commit doesn't attempt to handle that.

Search for SIGTSTP in section 2.4.3:

https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
This commit is contained in:
Lasse Collin 2023-01-26 18:29:17 +02:00
parent 3b1c8ac8d1
commit ff592c616e
4 changed files with 89 additions and 2 deletions

View File

@ -20,7 +20,12 @@
uint64_t opt_flush_timeout = 0; uint64_t opt_flush_timeout = 0;
#ifdef USE_SIGTSTP_HANDLER
static volatile uint64_t start_time;
#else
static uint64_t start_time; static uint64_t start_time;
#endif
static uint64_t next_flush; static uint64_t next_flush;
@ -48,10 +53,49 @@ mytime_now(void)
} }
#ifdef USE_SIGTSTP_HANDLER
extern void
mytime_sigtstp_handler(int sig lzma_attribute((__unused__)))
{
// Measure how long the process stays in the stopped state and add
// that amount to start_time. This way the the progress indicator
// won't count the stopped time as elapsed time and the estimated
// remaining time won't be confused by the time spent in the
// stopped state.
//
// FIXME? Is raising SIGSTOP the correct thing to do? POSIX.1-2017
// says that orphan processes shouldn't stop on SIGTSTP. So perhaps
// the most correct thing to do could be to revert to the default
// handler for SIGTSTP, unblock SIGTSTP, and then raise(SIGTSTP).
// It's quite a bit more complicated than just raising SIGSTOP though.
//
// The difference between raising SIGTSTP vs. SIGSTOP can be seen on
// the shell command line too by running "echo $?" after stopping
// a process but perhaps that doesn't matter.
const uint64_t t = mytime_now();
raise(SIGSTOP);
start_time += mytime_now() - t;
return;
}
#endif
extern void extern void
mytime_set_start_time(void) mytime_set_start_time(void)
{ {
#ifdef USE_SIGTSTP_HANDLER
// Block the signals when accessing start_time so that we cannot
// end up with a garbage value. start_time is volatile but access
// to it isn't atomic at least on 32-bit systems.
signals_block();
#endif
start_time = mytime_now(); start_time = mytime_now();
#ifdef USE_SIGTSTP_HANDLER
signals_unblock();
#endif
return; return;
} }
@ -59,7 +103,17 @@ mytime_set_start_time(void)
extern uint64_t extern uint64_t
mytime_get_elapsed(void) mytime_get_elapsed(void)
{ {
return mytime_now() - start_time; #ifdef USE_SIGTSTP_HANDLER
signals_block();
#endif
const uint64_t t = mytime_now() - start_time;
#ifdef USE_SIGTSTP_HANDLER
signals_unblock();
#endif
return t;
} }

View File

@ -21,6 +21,12 @@
extern uint64_t opt_flush_timeout; extern uint64_t opt_flush_timeout;
#ifdef USE_SIGTSTP_HANDLER
/// \brief Signal handler for SIGTSTP
extern void mytime_sigtstp_handler(int sig);
#endif
/// \brief Store the time when (de)compression was started /// \brief Store the time when (de)compression was started
/// ///
/// The start time is also stored as the time of the first flush. /// The start time is also stored as the time of the first flush.

View File

@ -49,6 +49,18 @@
# define ENABLE_SANDBOX 1 # define ENABLE_SANDBOX 1
#endif #endif
// Handling SIGTSTP keeps time-keeping for progress indicator correct
// if xz is stopped. It requires use of clock_gettime() as that is
// async-signal safe in POSIX. Require also SIGALRM support since
// on systems where SIGALRM isn't available, progress indicator code
// polls the time and the SIGTSTP handling adds slight overhead to
// that code. Most (all?) systems that have SIGTSTP also have SIGALRM
// so this requirement won't exclude many systems.
#if defined(HAVE_CLOCK_GETTIME) && defined(HAVE_CLOCK_MONOTONIC) \
&& defined(SIGTSTP) && defined(SIGALRM)
# define USE_SIGTSTP_HANDLER 1
#endif
#include "main.h" #include "main.h"
#include "mytime.h" #include "mytime.h"
#include "coder.h" #include "coder.h"

View File

@ -82,6 +82,11 @@ signals_init(void)
sigaddset(&hooked_signals, message_progress_sigs[i]); sigaddset(&hooked_signals, message_progress_sigs[i]);
#endif #endif
#ifdef USE_SIGTSTP_HANDLER
// Add the SIGTSTP handler from mytime.c to hooked_signals.
sigaddset(&hooked_signals, SIGTSTP);
#endif
// Using "my_sa" because "sa" may conflict with a sockaddr variable // Using "my_sa" because "sa" may conflict with a sockaddr variable
// from system headers on Solaris. // from system headers on Solaris.
struct sigaction my_sa; struct sigaction my_sa;
@ -96,10 +101,11 @@ signals_init(void)
my_sa.sa_flags = 0; my_sa.sa_flags = 0;
my_sa.sa_handler = &signal_handler; my_sa.sa_handler = &signal_handler;
struct sigaction old;
for (size_t i = 0; i < ARRAY_SIZE(sigs); ++i) { for (size_t i = 0; i < ARRAY_SIZE(sigs); ++i) {
// If the parent process has left some signals ignored, // If the parent process has left some signals ignored,
// we don't unignore them. // we don't unignore them.
struct sigaction old;
if (sigaction(sigs[i], NULL, &old) == 0 if (sigaction(sigs[i], NULL, &old) == 0
&& old.sa_handler == SIG_IGN) && old.sa_handler == SIG_IGN)
continue; continue;
@ -109,6 +115,15 @@ signals_init(void)
message_signal_handler(); message_signal_handler();
} }
#ifdef USE_SIGTSTP_HANDLER
if (!(sigaction(SIGTSTP, NULL, &old) == 0
&& old.sa_handler == SIG_IGN)) {
my_sa.sa_handler = &mytime_sigtstp_handler;
if (sigaction(SIGTSTP, &my_sa, NULL))
message_signal_handler();
}
#endif
signals_are_initialized = true; signals_are_initialized = true;
return; return;