[pacman-dev] Dropping SIGPIPE workaround from dload.c
Dave Reisner
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
> happening,
> * 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.
dR
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
> milliseconds
> 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
mailing list