diff mbox

[1/6] ALSA: seq-dummy: remove deadlock-causing events on close

Message ID 54C4F0E5.8030205@ladisch.de (mailing list archive)
State New, archived
Headers show

Commit Message

Clemens Ladisch Jan. 25, 2015, 1:34 p.m. UTC
When the last subscriber to a "Through" port has been removed, the
subscribed destination ports might still be active, so it would be
wrong to send "all sounds off" and "reset controller" events to them.
The proper place for such a shutdown would be the closing of the actual
MIDI port (and close_substream() in rawmidi.c already can do this).

This also fixes a deadlock when dummy_unuse() tries to send events to
its own port that is already locked because it is being freed.

Reported-by: Peter Billam <peter@www.pjb.com.au>
Fixes: 1da177e4c3f4
Cc: <stable@vger.kernel.org>
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
---
 sound/core/seq/seq_dummy.c |   31 -------------------------------
 1 file changed, 31 deletions(-)

Comments

Takashi Iwai Jan. 26, 2015, 12:53 p.m. UTC | #1
At Sun, 25 Jan 2015 14:34:29 +0100,
Clemens Ladisch wrote:
> 
> When the last subscriber to a "Through" port has been removed, the
> subscribed destination ports might still be active, so it would be
> wrong to send "all sounds off" and "reset controller" events to them.
> The proper place for such a shutdown would be the closing of the actual
> MIDI port (and close_substream() in rawmidi.c already can do this).
> 
> This also fixes a deadlock when dummy_unuse() tries to send events to
> its own port that is already locked because it is being freed.
> 
> Reported-by: Peter Billam <peter@www.pjb.com.au>
> Fixes: 1da177e4c3f4
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

Applied to for-linus branch, but I dropped the fixes tag that points
to 2.6.12-rc2, which is obviously not so useful.


thanks,

Takashi

> ---
>  sound/core/seq/seq_dummy.c |   31 -------------------------------
>  1 file changed, 31 deletions(-)
> 
> diff --git a/sound/core/seq/seq_dummy.c b/sound/core/seq/seq_dummy.c
> index ec667f1..5d905d9 100644
> --- a/sound/core/seq/seq_dummy.c
> +++ b/sound/core/seq/seq_dummy.c
> @@ -82,36 +82,6 @@ struct snd_seq_dummy_port {
>  static int my_client = -1;
> 
>  /*
> - * unuse callback - send ALL_SOUNDS_OFF and RESET_CONTROLLERS events
> - * to subscribers.
> - * Note: this callback is called only after all subscribers are removed.
> - */
> -static int
> -dummy_unuse(void *private_data, struct snd_seq_port_subscribe *info)
> -{
> -	struct snd_seq_dummy_port *p;
> -	int i;
> -	struct snd_seq_event ev;
> -
> -	p = private_data;
> -	memset(&ev, 0, sizeof(ev));
> -	if (p->duplex)
> -		ev.source.port = p->connect;
> -	else
> -		ev.source.port = p->port;
> -	ev.dest.client = SNDRV_SEQ_ADDRESS_SUBSCRIBERS;
> -	ev.type = SNDRV_SEQ_EVENT_CONTROLLER;
> -	for (i = 0; i < 16; i++) {
> -		ev.data.control.channel = i;
> -		ev.data.control.param = MIDI_CTL_ALL_SOUNDS_OFF;
> -		snd_seq_kernel_client_dispatch(p->client, &ev, 0, 0);
> -		ev.data.control.param = MIDI_CTL_RESET_CONTROLLERS;
> -		snd_seq_kernel_client_dispatch(p->client, &ev, 0, 0);
> -	}
> -	return 0;
> -}
> -
> -/*
>   * event input callback - just redirect events to subscribers
>   */
>  static int
> @@ -175,7 +145,6 @@ create_port(int idx, int type)
>  		| SNDRV_SEQ_PORT_TYPE_PORT;
>  	memset(&pcb, 0, sizeof(pcb));
>  	pcb.owner = THIS_MODULE;
> -	pcb.unuse = dummy_unuse;
>  	pcb.event_input = dummy_input;
>  	pcb.private_free = dummy_free;
>  	pcb.private_data = rec;
>
diff mbox

Patch

diff --git a/sound/core/seq/seq_dummy.c b/sound/core/seq/seq_dummy.c
index ec667f1..5d905d9 100644
--- a/sound/core/seq/seq_dummy.c
+++ b/sound/core/seq/seq_dummy.c
@@ -82,36 +82,6 @@  struct snd_seq_dummy_port {
 static int my_client = -1;

 /*
- * unuse callback - send ALL_SOUNDS_OFF and RESET_CONTROLLERS events
- * to subscribers.
- * Note: this callback is called only after all subscribers are removed.
- */
-static int
-dummy_unuse(void *private_data, struct snd_seq_port_subscribe *info)
-{
-	struct snd_seq_dummy_port *p;
-	int i;
-	struct snd_seq_event ev;
-
-	p = private_data;
-	memset(&ev, 0, sizeof(ev));
-	if (p->duplex)
-		ev.source.port = p->connect;
-	else
-		ev.source.port = p->port;
-	ev.dest.client = SNDRV_SEQ_ADDRESS_SUBSCRIBERS;
-	ev.type = SNDRV_SEQ_EVENT_CONTROLLER;
-	for (i = 0; i < 16; i++) {
-		ev.data.control.channel = i;
-		ev.data.control.param = MIDI_CTL_ALL_SOUNDS_OFF;
-		snd_seq_kernel_client_dispatch(p->client, &ev, 0, 0);
-		ev.data.control.param = MIDI_CTL_RESET_CONTROLLERS;
-		snd_seq_kernel_client_dispatch(p->client, &ev, 0, 0);
-	}
-	return 0;
-}
-
-/*
  * event input callback - just redirect events to subscribers
  */
 static int
@@ -175,7 +145,6 @@  create_port(int idx, int type)
 		| SNDRV_SEQ_PORT_TYPE_PORT;
 	memset(&pcb, 0, sizeof(pcb));
 	pcb.owner = THIS_MODULE;
-	pcb.unuse = dummy_unuse;
 	pcb.event_input = dummy_input;
 	pcb.private_free = dummy_free;
 	pcb.private_data = rec;