[vlc-devel] [PATCH] fix sigpipe crash and recover when bad network

[Janboe Ye]

Thanks

I will update my patch to avoid build break, and please help to review my patch again late.

> 2020年2月21日 下午6:19，Rémi Denis-Courmont <remi at remlab.net> 写道：
> 
> Hi,
> 
> Library code should not trigger SIGPIPE. But your patch breaks the build on most platforms. See the VLC network code how to block SIGPIPE correctly.
> 
> Le 21 février 2020 11:56:56 GMT+02:00, Janboe Ye <janboe.ye at outlook.com <mailto:janboe.ye at outlook.com>> a écrit :
> hi, 
> 
> Thanks for review. Is it better to ignore SIGPIPE in higher layer?
> 
> Yes, filter patch is trying to recovery playback by creating a fake EOS after ThreadRead fail to send data through broken socket, this would be better than keep trying to send data.
> 
> Any better suggestion?
> 
> Thanks a lot!
> 
>> 2020年2月21日 下午3:59，Rémi Denis-Courmont <remi at remlab.net <mailto:remi at remlab.net>> 写道：
>> 
>> Hi,
>> 
>> This looks like it'll break the build on all platforms but BSD derivatives. The filter patch has no rationale and looks like it'll cause spurious early EOS, thus breaking some streams.
>> 
>> Le 21 février 2020 09:33:46 GMT+02:00, Janboe Ye <janboe.ye at outlook.com <mailto:janboe.ye at outlook.com>> a écrit :
>> From 16540219ff8416395bd7994a6bb2fa9c5ab9984f Mon Sep 17 00:00:00 2001
>> From: Janboe Ye <janboe.ye at gmail.com <mailto:janboe.ye at gmail.com>>
>> Date: Fri, 21 Feb 2020 15:25:22 +0800
>> Subject: [PATCH] fix sigpipe crash and recover when bad network
>> 
>> dsm will send sigpipe when bad network conditional and crash app.
>> This will disable sigpipe and recovery after writing failure
>> 
>> Signed-off-by: Janboe Ye <janboe.ye at gmail.com <mailto:janboe.ye at gmail.com>>
>> .../0001-setsockopt-to-disable-sigpipe.patch  | 35 +++++++++++++++++++
>> contrib/src/libdsm/rules.mak                  |  1 +
>> modules/stream_filter/prefetch.c              |  6 ++--
>> 3 files changed, 39 insertions(+), 3 deletions(-)
>> create mode 100644 contrib/src/libdsm/0001-setsockopt-to-disable-sigpipe.patch
>> 
>> diff --git a/contrib/src/libdsm/0001-setsockopt-to-disable-sigpipe.patch b/contrib/src/libdsm/0001-setsockopt-to-disable-sigpipe.patch
>> new file mode 100644
>> index 0000000000..107c5f3f64
>> --- /dev/null
>> +++ b/contrib/src/libdsm/0001-setsockopt-to-disable-sigpipe.patch
>> @@ -0,0 +1,35 @@
>> +From b53b1242323c715cee018ac189b9863eccd95240 Mon Sep 17 00:00:00 2001
>> +From: Janboe Ye <janboe.ye at gmail.com <mailto:janboe.ye at gmail.com>>
>> +Date: Fri, 21 Feb 2020 15:14:32 +0800
>> +Subject: [PATCH] setsockopt to disable sigpipe
>> +
>> +this signal will crash app when network goes wrong
>> +
>> +Signed-off-by: Janboe Ye <janboe.ye at gmail.com <mailto:janboe.ye at gmail.com>>
>> +---
>> + src/netbios_session.c | 7 +++++++
>> + 1 file changed, 7 insertions(+)
>> +
>> +diff --git a/src/netbios_session.c b/src/netbios_session.c
>> +index a77cbc7..7ae4dd0 100644
>> +--- a/src/netbios_session.c
>> ++++ b/src/netbios_session.c
>> +@@ -54,8 +54,15 @@
>> + 
>> + static int open_socket_and_connect(netbios_session *s)
>> + {
>> ++    int optval = 0;
>> +     if ((s->socket = socket(AF_INET, SOCK_STREAM, 0)) < 0)
>> +         goto error;
>> ++
>> ++    //Never generate SIGPIPE on broken write
>> ++    optval = 1;
>> ++    if (setsockopt(s->socket, SOL_SOCKET, SO_NOSIGPIPE, (void *)&optval, sizeof(int)))
>> ++        goto error;
>> ++
>> +     if (connect(s->socket, (struct sockaddr *)&s->remote_addr, sizeof(s->remote_addr)) <0)
>> +         goto error;
>> + 
>> +-- 
>> +2.23.0
>> +
>> diff --git a/contrib/src/libdsm/rules.mak b/contrib/src/libdsm/rules.mak
>> index 1ea9b769b5..e7b9181865 100644
>> --- a/contrib/src/libdsm/rules.mak
>> +++ b/contrib/src/libdsm/rules.mak
>> @@ -24,6 +24,7 @@ libdsm: libdsm-$(LIBDSM_VERSION).tar.gz .sum-libdsm
>> 	$(APPLY) $(SRC)/libdsm/fix-pc-generation.patch
>> 	$(APPLY) $(SRC)/libdsm/fix-pipe-compat.patch
>> 	$(APPLY) $(SRC)/libdsm/0001-compat-Don-t-use-_pipe-when-building-for-winstore.patch
>> +	$(APPLY) $(SRC)/libdsm/0001-setsockopt-to-disable-sigpipe.patch
>> 	$(MOVE)
>> 
>> DEPS_libdsm = libtasn1 iconv
>> diff --git a/modules/stream_filter/prefetch.c b/modules/stream_filter/prefetch.c
>> index a31f52c67f..1ae63094ea 100644
>> --- a/modules/stream_filter/prefetch.c
>> +++ b/modules/stream_filter/prefetch.c
>> @@ -238,13 +238,13 @@ static void *Thread(void *data)
>>             len = sys->buffer_size - offset;
>> 
>>         ssize_t val = ThreadRead(stream, sys->buffer + offset, len);
>> -        if (val < 0)
>> -            continue;
>> -        if (val == 0)
>> +
>> +        if (val <= 0)
>>         {
>>             assert(len > 0);
>>             msg_Dbg(stream, "end of stream");
>>             sys->eof = true;
>> +            val = 0;
>>         }
>> 
>>         assert((size_t)val <= len);
>> 
>> -- 
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
> 
> 
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté._______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel <https://mailman.videolan.org/listinfo/vlc-devel>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200221/bff76180/attachment.html>