Linux-Bluetooth Archive on lore.kernel.org
 help / Atom feed
* [PATCH BlueZ] core: Catch SIGPIPE
@ 2018-11-17  6:00 Gal Ben-Haim
  2018-11-17 20:41 ` Gal Ben Haim
  0 siblings, 1 reply; 7+ messages in thread
From: Gal Ben-Haim @ 2018-11-17  6:00 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gal Ben-Haim

bluetoothd receives a SIGPIPE and terminates if writing to a pipe that
was acquired by AcquireNotify and there are no readers. it can be
reproduced by terminating the reader process without closing the reader
end of the pipe.

Catching SIGPIPE will cause the write call to return an EPIPE error
which will be logged as "io_send: Broken pipe".
---
 src/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/main.c b/src/main.c
index 4716f5388..54cdb8d3f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -533,6 +533,8 @@ static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
 	case SIGUSR2:
 		__btd_toggle_debug();
 		break;
+	case SIGPIPE:
+		break;
 	}
 
 	return TRUE;
@@ -549,6 +551,7 @@ static guint setup_signalfd(void)
 	sigaddset(&mask, SIGINT);
 	sigaddset(&mask, SIGTERM);
 	sigaddset(&mask, SIGUSR2);
+	sigaddset(&mask, SIGPIPE);
 
 	if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
 		perror("Failed to set signal mask");
-- 
2.19.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH BlueZ] core: Catch SIGPIPE
  2018-11-17  6:00 [PATCH BlueZ] core: Catch SIGPIPE Gal Ben-Haim
@ 2018-11-17 20:41 ` Gal Ben Haim
  2018-11-17 20:47   ` Gal Ben Haim
  0 siblings, 1 reply; 7+ messages in thread
From: Gal Ben Haim @ 2018-11-17 20:41 UTC (permalink / raw)
  To: linux-bluetooth

actually it does not log the broken pipe message, is it because errno
isn't set before the signal handler is done?
what do you think?
On Sat, Nov 17, 2018 at 8:00 AM Gal Ben-Haim <gbenhaim@augury.com> wrote:
>
> bluetoothd receives a SIGPIPE and terminates if writing to a pipe that
> was acquired by AcquireNotify and there are no readers. it can be
> reproduced by terminating the reader process without closing the reader
> end of the pipe.
>
> Catching SIGPIPE will cause the write call to return an EPIPE error
> which will be logged as "io_send: Broken pipe".
> ---
>  src/main.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/main.c b/src/main.c
> index 4716f5388..54cdb8d3f 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -533,6 +533,8 @@ static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
>         case SIGUSR2:
>                 __btd_toggle_debug();
>                 break;
> +       case SIGPIPE:
> +               break;
>         }
>
>         return TRUE;
> @@ -549,6 +551,7 @@ static guint setup_signalfd(void)
>         sigaddset(&mask, SIGINT);
>         sigaddset(&mask, SIGTERM);
>         sigaddset(&mask, SIGUSR2);
> +       sigaddset(&mask, SIGPIPE);
>
>         if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
>                 perror("Failed to set signal mask");
> --
> 2.19.1
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH BlueZ] core: Catch SIGPIPE
  2018-11-17 20:41 ` Gal Ben Haim
@ 2018-11-17 20:47   ` Gal Ben Haim
  2018-11-19  9:22     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 7+ messages in thread
From: Gal Ben Haim @ 2018-11-17 20:47 UTC (permalink / raw)
  To: linux-bluetooth

i don't think it works at all... bluetoothd is still exiting with SIGPIPE.
i'm a bit lost with fixing this, I tried few methods that were
suggested here and the only one that actually worked is to ignore the
signal.

please advise how do you want to proceed with this..

On Sat, Nov 17, 2018 at 10:41 PM Gal Ben Haim <gbenhaim@augury.com> wrote:
>
> actually it does not log the broken pipe message, is it because errno
> isn't set before the signal handler is done?
> what do you think?
> On Sat, Nov 17, 2018 at 8:00 AM Gal Ben-Haim <gbenhaim@augury.com> wrote:
> >
> > bluetoothd receives a SIGPIPE and terminates if writing to a pipe that
> > was acquired by AcquireNotify and there are no readers. it can be
> > reproduced by terminating the reader process without closing the reader
> > end of the pipe.
> >
> > Catching SIGPIPE will cause the write call to return an EPIPE error
> > which will be logged as "io_send: Broken pipe".
> > ---
> >  src/main.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/src/main.c b/src/main.c
> > index 4716f5388..54cdb8d3f 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -533,6 +533,8 @@ static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
> >         case SIGUSR2:
> >                 __btd_toggle_debug();
> >                 break;
> > +       case SIGPIPE:
> > +               break;
> >         }
> >
> >         return TRUE;
> > @@ -549,6 +551,7 @@ static guint setup_signalfd(void)
> >         sigaddset(&mask, SIGINT);
> >         sigaddset(&mask, SIGTERM);
> >         sigaddset(&mask, SIGUSR2);
> > +       sigaddset(&mask, SIGPIPE);
> >
> >         if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
> >                 perror("Failed to set signal mask");
> > --
> > 2.19.1
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH BlueZ] core: Catch SIGPIPE
  2018-11-17 20:47   ` Gal Ben Haim
@ 2018-11-19  9:22     ` Luiz Augusto von Dentz
  2018-11-19  9:47       ` Gal Ben Haim
  2018-11-19 13:27       ` Marcel Holtmann
  0 siblings, 2 replies; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-19  9:22 UTC (permalink / raw)
  To: gbenhaim; +Cc: linux-bluetooth

Hi Gal,

On Sat, Nov 17, 2018 at 10:48 PM Gal Ben Haim <gbenhaim@augury.com> wrote:
>
> i don't think it works at all... bluetoothd is still exiting with SIGPIPE.
> i'm a bit lost with fixing this, I tried few methods that were
> suggested here and the only one that actually worked is to ignore the
> signal.
>
> please advise how do you want to proceed with this..

We might be better of switching to socketpair instead of pipe2 then,
not only we can fix this problem using MSG_NOSIGNAL but also use
sendmsg and setting auxiliary data which can be used to set things
like the offset.

> On Sat, Nov 17, 2018 at 10:41 PM Gal Ben Haim <gbenhaim@augury.com> wrote:
> >
> > actually it does not log the broken pipe message, is it because errno
> > isn't set before the signal handler is done?
> > what do you think?
> > On Sat, Nov 17, 2018 at 8:00 AM Gal Ben-Haim <gbenhaim@augury.com> wrote:
> > >
> > > bluetoothd receives a SIGPIPE and terminates if writing to a pipe that
> > > was acquired by AcquireNotify and there are no readers. it can be
> > > reproduced by terminating the reader process without closing the reader
> > > end of the pipe.
> > >
> > > Catching SIGPIPE will cause the write call to return an EPIPE error
> > > which will be logged as "io_send: Broken pipe".
> > > ---
> > >  src/main.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/src/main.c b/src/main.c
> > > index 4716f5388..54cdb8d3f 100644
> > > --- a/src/main.c
> > > +++ b/src/main.c
> > > @@ -533,6 +533,8 @@ static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
> > >         case SIGUSR2:
> > >                 __btd_toggle_debug();
> > >                 break;
> > > +       case SIGPIPE:
> > > +               break;
> > >         }
> > >
> > >         return TRUE;
> > > @@ -549,6 +551,7 @@ static guint setup_signalfd(void)
> > >         sigaddset(&mask, SIGINT);
> > >         sigaddset(&mask, SIGTERM);
> > >         sigaddset(&mask, SIGUSR2);
> > > +       sigaddset(&mask, SIGPIPE);
> > >
> > >         if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
> > >                 perror("Failed to set signal mask");
> > > --
> > > 2.19.1
> > >



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH BlueZ] core: Catch SIGPIPE
  2018-11-19  9:22     ` Luiz Augusto von Dentz
@ 2018-11-19  9:47       ` Gal Ben Haim
  2018-11-19 10:59         ` Luiz Augusto von Dentz
  2018-11-19 13:27       ` Marcel Holtmann
  1 sibling, 1 reply; 7+ messages in thread
From: Gal Ben Haim @ 2018-11-19  9:47 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

are you suggesting to have a single socketpair for both notify_io and
write_io or 2x socketpair instead of 2x pipe2 ?

On Mon, Nov 19, 2018 at 11:22 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Gal,
>
> On Sat, Nov 17, 2018 at 10:48 PM Gal Ben Haim <gbenhaim@augury.com> wrote:
> >
> > i don't think it works at all... bluetoothd is still exiting with SIGPIPE.
> > i'm a bit lost with fixing this, I tried few methods that were
> > suggested here and the only one that actually worked is to ignore the
> > signal.
> >
> > please advise how do you want to proceed with this..
>
> We might be better of switching to socketpair instead of pipe2 then,
> not only we can fix this problem using MSG_NOSIGNAL but also use
> sendmsg and setting auxiliary data which can be used to set things
> like the offset.
>
> > On Sat, Nov 17, 2018 at 10:41 PM Gal Ben Haim <gbenhaim@augury.com> wrote:
> > >
> > > actually it does not log the broken pipe message, is it because errno
> > > isn't set before the signal handler is done?
> > > what do you think?
> > > On Sat, Nov 17, 2018 at 8:00 AM Gal Ben-Haim <gbenhaim@augury.com> wrote:
> > > >
> > > > bluetoothd receives a SIGPIPE and terminates if writing to a pipe that
> > > > was acquired by AcquireNotify and there are no readers. it can be
> > > > reproduced by terminating the reader process without closing the reader
> > > > end of the pipe.
> > > >
> > > > Catching SIGPIPE will cause the write call to return an EPIPE error
> > > > which will be logged as "io_send: Broken pipe".
> > > > ---
> > > >  src/main.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/src/main.c b/src/main.c
> > > > index 4716f5388..54cdb8d3f 100644
> > > > --- a/src/main.c
> > > > +++ b/src/main.c
> > > > @@ -533,6 +533,8 @@ static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
> > > >         case SIGUSR2:
> > > >                 __btd_toggle_debug();
> > > >                 break;
> > > > +       case SIGPIPE:
> > > > +               break;
> > > >         }
> > > >
> > > >         return TRUE;
> > > > @@ -549,6 +551,7 @@ static guint setup_signalfd(void)
> > > >         sigaddset(&mask, SIGINT);
> > > >         sigaddset(&mask, SIGTERM);
> > > >         sigaddset(&mask, SIGUSR2);
> > > > +       sigaddset(&mask, SIGPIPE);
> > > >
> > > >         if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
> > > >                 perror("Failed to set signal mask");
> > > > --
> > > > 2.19.1
> > > >
>
>
>
> --
> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH BlueZ] core: Catch SIGPIPE
  2018-11-19  9:47       ` Gal Ben Haim
@ 2018-11-19 10:59         ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-19 10:59 UTC (permalink / raw)
  To: gbenhaim; +Cc: linux-bluetooth

Im taking care of it, will send the patches when they are ready.
On Mon, Nov 19, 2018 at 11:47 AM Gal Ben Haim <gbenhaim@augury.com> wrote:
>
> are you suggesting to have a single socketpair for both notify_io and
> write_io or 2x socketpair instead of 2x pipe2 ?
>
> On Mon, Nov 19, 2018 at 11:22 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Gal,
> >
> > On Sat, Nov 17, 2018 at 10:48 PM Gal Ben Haim <gbenhaim@augury.com> wrote:
> > >
> > > i don't think it works at all... bluetoothd is still exiting with SIGPIPE.
> > > i'm a bit lost with fixing this, I tried few methods that were
> > > suggested here and the only one that actually worked is to ignore the
> > > signal.
> > >
> > > please advise how do you want to proceed with this..
> >
> > We might be better of switching to socketpair instead of pipe2 then,
> > not only we can fix this problem using MSG_NOSIGNAL but also use
> > sendmsg and setting auxiliary data which can be used to set things
> > like the offset.
> >
> > > On Sat, Nov 17, 2018 at 10:41 PM Gal Ben Haim <gbenhaim@augury.com> wrote:
> > > >
> > > > actually it does not log the broken pipe message, is it because errno
> > > > isn't set before the signal handler is done?
> > > > what do you think?
> > > > On Sat, Nov 17, 2018 at 8:00 AM Gal Ben-Haim <gbenhaim@augury.com> wrote:
> > > > >
> > > > > bluetoothd receives a SIGPIPE and terminates if writing to a pipe that
> > > > > was acquired by AcquireNotify and there are no readers. it can be
> > > > > reproduced by terminating the reader process without closing the reader
> > > > > end of the pipe.
> > > > >
> > > > > Catching SIGPIPE will cause the write call to return an EPIPE error
> > > > > which will be logged as "io_send: Broken pipe".
> > > > > ---
> > > > >  src/main.c | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/src/main.c b/src/main.c
> > > > > index 4716f5388..54cdb8d3f 100644
> > > > > --- a/src/main.c
> > > > > +++ b/src/main.c
> > > > > @@ -533,6 +533,8 @@ static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
> > > > >         case SIGUSR2:
> > > > >                 __btd_toggle_debug();
> > > > >                 break;
> > > > > +       case SIGPIPE:
> > > > > +               break;
> > > > >         }
> > > > >
> > > > >         return TRUE;
> > > > > @@ -549,6 +551,7 @@ static guint setup_signalfd(void)
> > > > >         sigaddset(&mask, SIGINT);
> > > > >         sigaddset(&mask, SIGTERM);
> > > > >         sigaddset(&mask, SIGUSR2);
> > > > > +       sigaddset(&mask, SIGPIPE);
> > > > >
> > > > >         if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
> > > > >                 perror("Failed to set signal mask");
> > > > > --
> > > > > 2.19.1
> > > > >
> >
> >
> >
> > --
> > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH BlueZ] core: Catch SIGPIPE
  2018-11-19  9:22     ` Luiz Augusto von Dentz
  2018-11-19  9:47       ` Gal Ben Haim
@ 2018-11-19 13:27       ` Marcel Holtmann
  1 sibling, 0 replies; 7+ messages in thread
From: Marcel Holtmann @ 2018-11-19 13:27 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: gbenhaim, linux-bluetooth

Hi Luiz,

>> i don't think it works at all... bluetoothd is still exiting with SIGPIPE.
>> i'm a bit lost with fixing this, I tried few methods that were
>> suggested here and the only one that actually worked is to ignore the
>> signal.
>> 
>> please advise how do you want to proceed with this..
> 
> We might be better of switching to socketpair instead of pipe2 then,
> not only we can fix this problem using MSG_NOSIGNAL but also use
> sendmsg and setting auxiliary data which can be used to set things
> like the offset.

I agree, lets switch to socketpair. I have read up on handling SIGPIPE via signalfd and other means and it is just complicated.

Also we should include MSG_NOSIGNAL in all of our code and even switch the simple read/write to it.

Regards

Marcel


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-17  6:00 [PATCH BlueZ] core: Catch SIGPIPE Gal Ben-Haim
2018-11-17 20:41 ` Gal Ben Haim
2018-11-17 20:47   ` Gal Ben Haim
2018-11-19  9:22     ` Luiz Augusto von Dentz
2018-11-19  9:47       ` Gal Ben Haim
2018-11-19 10:59         ` Luiz Augusto von Dentz
2018-11-19 13:27       ` Marcel Holtmann

Linux-Bluetooth Archive on lore.kernel.org

Archives are clonable: git clone --mirror https://lore.kernel.org/linux-bluetooth/0 linux-bluetooth/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-bluetooth linux-bluetooth/ https://lore.kernel.org/linux-bluetooth \
		linux-bluetooth@vger.kernel.org linux-bluetooth@archiver.kernel.org
	public-inbox-index linux-bluetooth


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-bluetooth


AGPL code for this site: git clone https://public-inbox.org/ public-inbox