[pacman-dev] Dropping SIGPIPE workaround from dload.c
d at falconindy.com
Mon Apr 20 23:36:04 UTC 2020
On Mon, Apr 20, 2020, 18:04 Anatol Pomozov <anatol.pomozov at gmail.com> wrote:
> Hi folks
> I am looking at the lib/libalpm/dload.c download codepath and found
> that it has a following snippet:
> /* Ignore any SIGPIPE signals. With libcurl, these shouldn't be
> * but better safe than sorry. Store the old signal handler first.
Yup, this is fine to remove. Curl would actually signal DNS timeouts with
SIGALRM not SIGPIPE. This is leftover crap from the libfetch days.
mask_signal(SIGPIPE, SIG_IGN, &orig_sig_pipe);
> dload_interrupted = 0;
> mask_signal(SIGINT, &inthandler, &orig_sig_int);
> My understanding that this code tries to handle DNS timeouts that are
> signaled as a SIGPIPE. And to avoid killing the whole app this code
> installs this sighandler.
> But my experiment (at my dev branch that removes the codesnippet
> above) shows that curl handles DNS timeout correctly:
> sudo pacman -U http://blackhole.webpagetest.org/foo.zst
> :: Retrieving packages...
> foo.zst failed to download
> foo.zst.sig failed to download
> error: failed retrieving file 'foo.zst' from blackhole.webpagetest.org
> : Connection timed out after 10000 milliseconds
> error: failed retrieving file 'foo.zst.sig' from
> blackhole.webpagetest.org : Connection timed out after 10000
> warning: failed to retrieve some files
> There is also a test that checks for SIGPIPE handling
> (test/pacman/tests/scriptlet-signal-reset.py) and it passes as well:
> 157/332 scriptlet-signal-reset.py OK
> 0.11832118034362793 s
> Which makes me believe that SIGPIPE handler trick is not needed and
> mCURL handles timeouts correctly.
> I've been using a branch without this handler for two months and never
> seen any signal related issues with my parallel-download pacman.
> I am thinking of *not* carrying this workaround to the new multiplexed
> function and want to check with you if you are OK with it.
More information about the pacman-dev