[pacman-dev] [PATCH] alpm: Fix SIGINT handling re: aborting download
Upon receiving SIGINT a flag is set to abort the (curl) download. However, since it was never reset/initialized, if a front-end doesn't actually exit on SIGINT, and later tries any operation that needs to perform a new download, said download would always get aborted right away due to the flag not having been reset. Signed-off-by: Olivier Brunel <jjk@jjacky.com> --- lib/libalpm/dload.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index cca39470..0a3293cf 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -253,6 +253,7 @@ static void curl_set_handle_opts(struct dload_payload *payload, const char *useragent = getenv("HTTP_USER_AGENT"); struct stat st; + dload_interrupted = 0; /* the curl_easy handle is initialized with the alpm handle, so we only need * to reset the handle's parameters for each time it's used. */ curl_easy_reset(curl); -- 2.19.0
Variable dload_interrupted is used both to abort a download because SIGINT was caught, and when a file limit is reached. But raising SIGINT is only meant to happen in the first case. Signed-off-by: Olivier Brunel <jjk@jjacky.com> --- lib/libalpm/dload.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 0a3293cf..c70554b8 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -586,7 +586,7 @@ cleanup: unmask_signal(SIGINT, &orig_sig_int); unmask_signal(SIGPIPE, &orig_sig_pipe); /* if we were interrupted, trip the old handler */ - if(dload_interrupted) { + if(dload_interrupted == ABORT_SIGINT) { raise(SIGINT); } -- 2.19.0
On 10/09/18 at 06:29pm, Olivier Brunel wrote:
Variable dload_interrupted is used both to abort a download because SIGINT was caught, and when a file limit is reached. But raising SIGINT is only meant to happen in the first case.
Signed-off-by: Olivier Brunel <jjk@jjacky.com> ---
ACK.
On 10/09/18 at 06:29pm, Olivier Brunel wrote:
Upon receiving SIGINT a flag is set to abort the (curl) download. However, since it was never reset/initialized, if a front-end doesn't actually exit on SIGINT, and later tries any operation that needs to perform a new download, said download would always get aborted right away due to the flag not having been reset.
Signed-off-by: Olivier Brunel <jjk@jjacky.com> --- lib/libalpm/dload.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index cca39470..0a3293cf 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -253,6 +253,7 @@ static void curl_set_handle_opts(struct dload_payload *payload, const char *useragent = getenv("HTTP_USER_AGENT"); struct stat st;
+ dload_interrupted = 0;
I think set_handle_opts is the wrong place to reset it since it is in fact not a handle option. Let's put it right before the signal handlers are registered to keep signal-related things together.
/* the curl_easy handle is initialized with the alpm handle, so we only need * to reset the handle's parameters for each time it's used. */ curl_easy_reset(curl); -- 2.19.0
Upon receiving SIGINT a flag is set to abort the (curl) download. However, since it was never reset/initialized, if a front-end doesn't actually exit on SIGINT, and later tries any operation that needs to perform a new download, said download would always get aborted right away due to the flag not having been reset. --- v2: Correctly reset variable before setting signal handler Signed-off-by: Olivier Brunel <jjk@jjacky.com> --- lib/libalpm/dload.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index cca39470..4d0adb19 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -431,6 +431,7 @@ static int curl_download_internal(struct dload_payload *payload, /* Ignore any SIGPIPE signals. With libcurl, these shouldn't be happening, * but better safe than sorry. Store the old signal handler first. */ mask_signal(SIGPIPE, SIG_IGN, &orig_sig_pipe); + dload_interrupted = 0; mask_signal(SIGINT, &inthandler, &orig_sig_int); /* perform transfer */ -- 2.19.0
participants (2)
-
Andrew Gregory
-
Olivier Brunel