[sound]: gnome-control-center crashed with SIGABRT in pa_memblock_acquire()
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
pulseaudio (Ubuntu) |
Fix Released
|
Medium
|
David Henningsson |
Bug Description
The Sound panel has been extremely crash-prone for the past oh, maybe 2 weeks.
ProblemType: Crash
DistroRelease: Ubuntu 12.10
Package: gnome-control-
ProcVersionSign
Uname: Linux 3.5.0-15-generic x86_64
ApportVersion: 2.5.2-0ubuntu4
Architecture: amd64
CrashCounter: 1
Date: Wed Sep 26 15:34:20 2012
ExecutablePath: /usr/bin/
ProcCmdline: gnome-control-
Signal: 6
SourcePackage: gnome-control-
StacktraceTop:
raise () from /lib/x86_
abort () from /lib/x86_
pa_memblock_
pa_stream_peek () from /usr/lib/
?? () from /usr/lib/
Title: [sound]: gnome-control-
UpgradeStatus: No upgrade log present (probably fresh install)
UserGroups: adm cdrom dip libvirtd lpadmin plugdev sambashare sbuild sudo
usr_lib_
activity-
deja-dup 23.92-0ubuntu1
gnome-
indicator-datetime 12.10.1-0ubuntu1
Related branches
Jeremy Bícha (jbicha) wrote : | #1 |
- Dependencies.txt Edit (12.2 KiB, text/plain; charset="utf-8")
- Disassembly.txt Edit (854 bytes, text/plain; charset="utf-8")
- ProcEnviron.txt Edit (280 bytes, text/plain; charset="utf-8")
- ProcMaps.txt Edit (109.3 KiB, text/plain; charset="utf-8")
- ProcStatus.txt Edit (830 bytes, text/plain; charset="utf-8")
- Registers.txt Edit (731 bytes, text/plain; charset="utf-8")
- Stacktrace.txt Edit (2.1 KiB, text/plain; charset="utf-8")
- ThreadStacktrace.txt Edit (3.6 KiB, text/plain; charset="utf-8")
Apport retracing service (apport) wrote : | #2 |
Apport retracing service (apport) wrote : Stacktrace.txt | #3 |
Apport retracing service (apport) wrote : StacktraceSource.txt | #4 |
Apport retracing service (apport) wrote : ThreadStacktrace.txt | #5 |
Changed in gnome-control-center (Ubuntu): | |
importance: | Undecided → Medium |
summary: |
- [sound]: gnome-control-center crashed with SIGABRT in raise() + [sound]: gnome-control-center crashed with SIGABRT in + pa_memblock_acquire() |
tags: | removed: need-amd64-retrace |
Sebastien Bacher (seb128) wrote : | #6 |
Hum, that stacktrace points to pulseaudio, and the only update which matches that timeframe is:
https:/
could you try if the issue still happen with the previous version?
visibility: | private → public |
affects: | gnome-control-center (Ubuntu) → pulseaudio (Ubuntu) |
David Henningsson (diwic) wrote : [PATCH] stream: Return error in case a client peeks to early | #7 |
If there is no silence memblock and no data, pa_memblockq_peek can
return NULL. In this case, do not crash on an assertion in
pa_memblock_
BugLink: http://
Signed-off-by: David Henningsson <email address hidden>
---
src/pulse/stream.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/pulse/
index 2b6d306..9bb0995 100644
--- a/src/pulse/
+++ b/src/pulse/
@@ -1598,6 +1598,8 @@ int pa_stream_
return 0;
}
+ PA_CHECK_
+
}
--
1.7.9.5
Changed in pulseaudio (Ubuntu): | |
status: | New → In Progress |
assignee: | nobody → David Henningsson (diwic) |
Tanu Kaskinen (tanuk) wrote : Re: [pulseaudio-discuss] [PATCH] stream: Return error in case a client peeks to early | #8 |
On Mon, 2012-10-01 at 17:06 +0200, David Henningsson wrote:
> If there is no silence memblock and no data, pa_memblockq_peek can
> return NULL. In this case, do not crash on an assertion in
> pa_memblock_
If there is no data in the buffer, pa_stream_peek() is supposed to
return NULL according to the documentation. And it does that: if there's
no data, pa_memblock_peek() will return a negative value, causing
pa_stream_peek() to return NULL.
The problem is the case where the buffer does contain data, but not at
the read index. That is, there is a hole in the buffer. The client
documentation doesn't have any warnings about holes, so the only safe
way to handle holes is to return silence. Fixing this should be a simple
matter of giving a silence memchunk when creating record_memblockq.
--
Tanu
David Henningsson (diwic) wrote : | #9 |
On 10/02/2012 10:38 PM, Tanu Kaskinen wrote:
> On Mon, 2012-10-01 at 17:06 +0200, David Henningsson wrote:
>> If there is no silence memblock and no data, pa_memblockq_peek can
>> return NULL. In this case, do not crash on an assertion in
>> pa_memblock_
>
> If there is no data in the buffer, pa_stream_peek() is supposed to
> return NULL according to the documentation. And it does that: if there's
> no data, pa_memblock_peek() will return a negative value, causing
> pa_stream_peek() to return NULL.
>
> The problem is the case where the buffer does contain data, but not at
> the read index. That is, there is a hole in the buffer. The client
> documentation doesn't have any warnings about holes, so the only safe
> way to handle holes is to return silence. Fixing this should be a simple
> matter of giving a silence memchunk when creating record_memblockq.
I'm not so sure. Silence, as in all zeroes, might work for S16 audio
data, but what about other formats? Compressed audio? Peak audio (which
I think is the case here)? Etc.
Also maybe it could also be valuable for the client to distinguish
between no data available, and valid zero data.
How about returning NULL and adding to the documentation something like:
-If no data is available this will return a NULL pointer.
+If no data is available (at the current read position), this will
return a NULL pointer.
--
David Henningsson, Canonical Ltd.
https:/
David Henningsson (diwic) wrote : [PATCH v2] stream: Return error in case a client peeks to early | #10 |
If there is no silence memblock and no data, pa_memblockq_peek can
return NULL. In this case, do not crash on an assertion in
pa_memblock_
BugLink: http://
Signed-off-by: David Henningsson <email address hidden>
---
src/pulse/stream.c | 3 ++-
src/pulse/stream.h | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/pulse/
index 2b6d306..0defb4f 100644
--- a/src/pulse/
+++ b/src/pulse/
@@ -1592,7 +1592,8 @@ int pa_stream_
if (!s->peek_
- if (pa_memblockq_
+ if (pa_memblockq_
+ !s->peek_
*data = NULL;
return 0;
diff --git a/src/pulse/
index b4464fa..8665d13 100644
--- a/src/pulse/
+++ b/src/pulse/
@@ -537,8 +537,8 @@ int pa_stream_write(
* \a data will point to the actual data and \a nbytes will contain the size
* of the data in bytes (which can be less or more than a complete
* fragment). Use pa_stream_drop() to actually remove the data from
- * the buffer. If no data is available this will return a NULL
- * pointer. */
+ * the buffer. If no data is available (at the current read position)
+ * this will return a NULL pointer. */
int pa_stream_peek(
pa_stream *p /**< The stream to use */,
const void **data /**< Pointer to pointer that will point to data */,
--
1.7.9.5
Launchpad Janitor (janitor) wrote : | #11 |
This bug was fixed in the package pulseaudio - 1:2.1-0ubuntu4
---------------
pulseaudio (1:2.1-0ubuntu4) quantal-proposed; urgency=low
* 0101-alsa-
Fix muted audio on startup in Virtualbox VM (LP: #1016969)
* 0020-stream-
Fix clients crashing when asking for data, but there is no data
to hand out (LP: #1058200)
-- David Henningsson <email address hidden> Thu, 04 Oct 2012 14:43:27 +0200
Changed in pulseaudio (Ubuntu): | |
status: | In Progress → Fix Released |
Tanu Kaskinen (tanuk) wrote : Re: [pulseaudio-discuss] [PATCH] stream: Return error in case a client peeks to early | #12 |
On Wed, 2012-10-03 at 08:50 +0200, David Henningsson wrote:
> On 10/02/2012 10:38 PM, Tanu Kaskinen wrote:
> > On Mon, 2012-10-01 at 17:06 +0200, David Henningsson wrote:
> >> If there is no silence memblock and no data, pa_memblockq_peek can
> >> return NULL. In this case, do not crash on an assertion in
> >> pa_memblock_
> >
> > If there is no data in the buffer, pa_stream_peek() is supposed to
> > return NULL according to the documentation. And it does that: if there's
> > no data, pa_memblock_peek() will return a negative value, causing
> > pa_stream_peek() to return NULL.
> >
> > The problem is the case where the buffer does contain data, but not at
> > the read index. That is, there is a hole in the buffer. The client
> > documentation doesn't have any warnings about holes, so the only safe
> > way to handle holes is to return silence. Fixing this should be a simple
> > matter of giving a silence memchunk when creating record_memblockq.
>
> I'm not so sure. Silence, as in all zeroes, might work for S16 audio
> data, but what about other formats? Compressed audio? Peak audio (which
> I think is the case here)? Etc.
Good point. Regarding PCM, if pa_memchunk_
function will take care of filling the memory with appropriate content.
But that doesn't work with compressed audio.
> Also maybe it could also be valuable for the client to distinguish
> between no data available, and valid zero data.
>
> How about returning NULL and adding to the documentation something like:
>
> -If no data is available this will return a NULL pointer.
> +If no data is available (at the current read position), this will
> return a NULL pointer.
An addition: the client probably wants to know how large the hole is. It
might be possible to figure that out somehow from the read index, but I
think it would make sense to return the hole size in the length
parameter.
--
Tanu
David Henningsson (diwic) wrote : Re: [PATCH] stream: Return error in case a client peeks to early | #13 |
2012-11-03 17:19, Colin Guthrie skrev:
> 'Twas brillig, and Tanu Kaskinen at 05/10/12 13:58 did gyre and gimble:
>> On Wed, 2012-10-03 at 08:50 +0200, David Henningsson wrote:
>>> On 10/02/2012 10:38 PM, Tanu Kaskinen wrote:
>>>> On Mon, 2012-10-01 at 17:06 +0200, David Henningsson wrote:
>>>>> If there is no silence memblock and no data, pa_memblockq_peek can
>>>>> return NULL. In this case, do not crash on an assertion in
>>>>> pa_memblock_
>>>> If there is no data in the buffer, pa_stream_peek() is supposed to
>>>> return NULL according to the documentation. And it does that: if there's
>>>> no data, pa_memblock_peek() will return a negative value, causing
>>>> pa_stream_peek() to return NULL.
>>>>
>>>> The problem is the case where the buffer does contain data, but not at
>>>> the read index. That is, there is a hole in the buffer. The client
>>>> documentation doesn't have any warnings about holes, so the only safe
>>>> way to handle holes is to return silence. Fixing this should be a simple
>>>> matter of giving a silence memchunk when creating record_memblockq.
>>> I'm not so sure. Silence, as in all zeroes, might work for S16 audio
>>> data, but what about other formats? Compressed audio? Peak audio (which
>>> I think is the case here)? Etc.
>> Good point. Regarding PCM, if pa_memchunk_
>> function will take care of filling the memory with appropriate content.
>> But that doesn't work with compressed audio.
>>
>>> Also maybe it could also be valuable for the client to distinguish
>>> between no data available, and valid zero data.
>>>
>>> How about returning NULL and adding to the documentation something like:
>>>
>>> -If no data is available this will return a NULL pointer.
>>> +If no data is available (at the current read position), this will
>>> return a NULL pointer.
>> An addition: the client probably wants to know how large the hole is. It
>> might be possible to figure that out somehow from the read index, but I
>> think it would make sense to return the hole size in the length
>> parameter.
> This discussion seemed to stagnate. Is this worth fixing/documenting for
> the 3.0 release?
>
> Col
>
>
Returning NULL seems to be the right thing to do here, even if
gnome-control-
need an additional patch in g-c-c.
So assuming I commit a patch doing that. If somebody else wants to add
logic to figure out how large the hole is, that could be discussed
separately.
Any objections?
--
David Henningsson, Canonical Ltd.
https:/
Tanu Kaskinen (tanuk) wrote : Re: [pulseaudio-discuss] [PATCH] stream: Return error in case a client peeks to early | #14 |
On Sat, 2012-11-03 at 19:51 +0100, David Henningsson wrote:
> 2012-11-03 17:19, Colin Guthrie skrev:
> > 'Twas brillig, and Tanu Kaskinen at 05/10/12 13:58 did gyre and gimble:
> >> On Wed, 2012-10-03 at 08:50 +0200, David Henningsson wrote:
> >>> On 10/02/2012 10:38 PM, Tanu Kaskinen wrote:
> >>>> On Mon, 2012-10-01 at 17:06 +0200, David Henningsson wrote:
> >>>>> If there is no silence memblock and no data, pa_memblockq_peek can
> >>>>> return NULL. In this case, do not crash on an assertion in
> >>>>> pa_memblock_
> >>>> If there is no data in the buffer, pa_stream_peek() is supposed to
> >>>> return NULL according to the documentation. And it does that: if there's
> >>>> no data, pa_memblock_peek() will return a negative value, causing
> >>>> pa_stream_peek() to return NULL.
> >>>>
> >>>> The problem is the case where the buffer does contain data, but not at
> >>>> the read index. That is, there is a hole in the buffer. The client
> >>>> documentation doesn't have any warnings about holes, so the only safe
> >>>> way to handle holes is to return silence. Fixing this should be a simple
> >>>> matter of giving a silence memchunk when creating record_memblockq.
> >>> I'm not so sure. Silence, as in all zeroes, might work for S16 audio
> >>> data, but what about other formats? Compressed audio? Peak audio (which
> >>> I think is the case here)? Etc.
> >> Good point. Regarding PCM, if pa_memchunk_
> >> function will take care of filling the memory with appropriate content.
> >> But that doesn't work with compressed audio.
> >>
> >>> Also maybe it could also be valuable for the client to distinguish
> >>> between no data available, and valid zero data.
> >>>
> >>> How about returning NULL and adding to the documentation something like:
> >>>
> >>> -If no data is available this will return a NULL pointer.
> >>> +If no data is available (at the current read position), this will
> >>> return a NULL pointer.
> >> An addition: the client probably wants to know how large the hole is. It
> >> might be possible to figure that out somehow from the read index, but I
> >> think it would make sense to return the hole size in the length
> >> parameter.
> > This discussion seemed to stagnate. Is this worth fixing/documenting for
> > the 3.0 release?
> >
> > Col
> >
> >
> Returning NULL seems to be the right thing to do here, even if
> gnome-control-
> need an additional patch in g-c-c.
> So assuming I commit a patch doing that. If somebody else wants to add
> logic to figure out how large the hole is, that could be discussed
> separately.
> Any objections?
It's not clear what you meant by "add logic to figure out how large the
hole is". Add to where? pa_stream_peek() or gnome-control-
To me, reporting the hole length in the "nbytes" parameter of
pa_stream_peek() seems like the right thing to do, so I hope your patch
will do this.
--
Tanu
David Henningsson (diwic) wrote : | #15 |
On 11/04/2012 02:22 PM, Tanu Kaskinen wrote:
> On Sat, 2012-11-03 at 19:51 +0100, David Henningsson wrote:
>> 2012-11-03 17:19, Colin Guthrie skrev:
>>> 'Twas brillig, and Tanu Kaskinen at 05/10/12 13:58 did gyre and gimble:
>>>> On Wed, 2012-10-03 at 08:50 +0200, David Henningsson wrote:
>>>>> On 10/02/2012 10:38 PM, Tanu Kaskinen wrote:
>>>>>> On Mon, 2012-10-01 at 17:06 +0200, David Henningsson wrote:
>>>>>>> If there is no silence memblock and no data, pa_memblockq_peek can
>>>>>>> return NULL. In this case, do not crash on an assertion in
>>>>>>> pa_memblock_
>>>>>> If there is no data in the buffer, pa_stream_peek() is supposed to
>>>>>> return NULL according to the documentation. And it does that: if there's
>>>>>> no data, pa_memblock_peek() will return a negative value, causing
>>>>>> pa_stream_peek() to return NULL.
>>>>>>
>>>>>> The problem is the case where the buffer does contain data, but not at
>>>>>> the read index. That is, there is a hole in the buffer. The client
>>>>>> documentation doesn't have any warnings about holes, so the only safe
>>>>>> way to handle holes is to return silence. Fixing this should be a simple
>>>>>> matter of giving a silence memchunk when creating record_memblockq.
>>>>> I'm not so sure. Silence, as in all zeroes, might work for S16 audio
>>>>> data, but what about other formats? Compressed audio? Peak audio (which
>>>>> I think is the case here)? Etc.
>>>> Good point. Regarding PCM, if pa_memchunk_
>>>> function will take care of filling the memory with appropriate content.
>>>> But that doesn't work with compressed audio.
>>>>
>>>>> Also maybe it could also be valuable for the client to distinguish
>>>>> between no data available, and valid zero data.
>>>>>
>>>>> How about returning NULL and adding to the documentation something like:
>>>>>
>>>>> -If no data is available this will return a NULL pointer.
>>>>> +If no data is available (at the current read position), this will
>>>>> return a NULL pointer.
>>>> An addition: the client probably wants to know how large the hole is. It
>>>> might be possible to figure that out somehow from the read index, but I
>>>> think it would make sense to return the hole size in the length
>>>> parameter.
>>> This discussion seemed to stagnate. Is this worth fixing/documenting for
>>> the 3.0 release?
>>>
>> Returning NULL seems to be the right thing to do here, even if
>> gnome-control-
>> need an additional patch in g-c-c.
>> So assuming I commit a patch doing that. If somebody else wants to add
>> logic to figure out how large the hole is, that could be discussed
>> separately.
>> Any objections?
>
> It's not clear what you meant by "add logic to figure out how large the
> hole is". Add to where? pa_stream_peek() or gnome-control-
I was referring to your earlier comment "An addition: the client
probably wants to know how large the hole is.", i e pa_stream_peek.
> To me, reporting the hole length in the "nbytes" parameter of
> pa_stream_peek() seems like the right thing to do, so I hope your patch
> will do this.
It do...
Tanu Kaskinen (tanuk) wrote : | #16 |
On Sun, 2012-11-04 at 23:09 +0100, David Henningsson wrote:
> On 11/04/2012 02:22 PM, Tanu Kaskinen wrote:
> > On Sat, 2012-11-03 at 19:51 +0100, David Henningsson wrote:
> >> Returning NULL seems to be the right thing to do here, even if
> >> gnome-control-
> >> need an additional patch in g-c-c.
> >> So assuming I commit a patch doing that. If somebody else wants to add
> >> logic to figure out how large the hole is, that could be discussed
> >> separately.
> >> Any objections?
> >
> > It's not clear what you meant by "add logic to figure out how large the
> > hole is". Add to where? pa_stream_peek() or gnome-control-
>
> I was referring to your earlier comment "An addition: the client
> probably wants to know how large the hole is.", i e pa_stream_peek.
>
> > To me, reporting the hole length in the "nbytes" parameter of
> > pa_stream_peek() seems like the right thing to do, so I hope your patch
> > will do this.
>
> It does not: I was just suggesting to discuss that separately.
Ok. Well, I'm suggesting to not make these issues separate, since it's a
small thing to decide and implement, and doing a separate fix would be
"fixing a fix". Does someone have an issue with returning the hole size
in the nbytes argument?
--
Tanu
David Henningsson (diwic) wrote : | #17 |
On 11/05/2012 07:13 PM, Tanu Kaskinen wrote:
> On Sun, 2012-11-04 at 23:09 +0100, David Henningsson wrote:
>> On 11/04/2012 02:22 PM, Tanu Kaskinen wrote:
>>> On Sat, 2012-11-03 at 19:51 +0100, David Henningsson wrote:
>>>> Returning NULL seems to be the right thing to do here, even if
>>>> gnome-control-
>>>> need an additional patch in g-c-c.
>>>> So assuming I commit a patch doing that. If somebody else wants to add
>>>> logic to figure out how large the hole is, that could be discussed
>>>> separately.
>>>> Any objections?
>>>
>>> It's not clear what you meant by "add logic to figure out how large the
>>> hole is". Add to where? pa_stream_peek() or gnome-control-
>>
>> I was referring to your earlier comment "An addition: the client
>> probably wants to know how large the hole is.", i e pa_stream_peek.
>>
>>> To me, reporting the hole length in the "nbytes" parameter of
>>> pa_stream_peek() seems like the right thing to do, so I hope your patch
>>> will do this.
>>
>> It does not: I was just suggesting to discuss that separately.
>
> Ok. Well, I'm suggesting to not make these issues separate, since it's a
> small thing to decide and implement, and doing a separate fix would be
> "fixing a fix".
I don't have an issue with step-by-step improvements. I see this as such
(if the second step is to be seen as an improvement).
> Does someone have an issue with returning the hole size
> in the nbytes argument?
Not if you're volunteering to do the work. All I care about is that PA
clients can crash for this reason, and I'd like that to stop happening.
Note: gnome-control-
currently checks the length rather than the NULL pointer, IIRC.
--
David Henningsson, Canonical Ltd.
https:/
Tanu Kaskinen (tanuk) wrote : | #18 |
On Tue, 2012-11-06 at 08:36 +0100, David Henningsson wrote:
> On 11/05/2012 07:13 PM, Tanu Kaskinen wrote:
> > On Sun, 2012-11-04 at 23:09 +0100, David Henningsson wrote:
> >> On 11/04/2012 02:22 PM, Tanu Kaskinen wrote:
> >>> On Sat, 2012-11-03 at 19:51 +0100, David Henningsson wrote:
> >>>> Returning NULL seems to be the right thing to do here, even if
> >>>> gnome-control-
> >>>> need an additional patch in g-c-c.
> >>>> So assuming I commit a patch doing that. If somebody else wants to add
> >>>> logic to figure out how large the hole is, that could be discussed
> >>>> separately.
> >>>> Any objections?
> >>>
> >>> It's not clear what you meant by "add logic to figure out how large the
> >>> hole is". Add to where? pa_stream_peek() or gnome-control-
> >>
> >> I was referring to your earlier comment "An addition: the client
> >> probably wants to know how large the hole is.", i e pa_stream_peek.
> >>
> >>> To me, reporting the hole length in the "nbytes" parameter of
> >>> pa_stream_peek() seems like the right thing to do, so I hope your patch
> >>> will do this.
> >>
> >> It does not: I was just suggesting to discuss that separately.
> >
> > Ok. Well, I'm suggesting to not make these issues separate, since it's a
> > small thing to decide and implement, and doing a separate fix would be
> > "fixing a fix".
>
> I don't have an issue with step-by-step improvements. I see this as such
> (if the second step is to be seen as an improvement).
>
> > Does someone have an issue with returning the hole size
> > in the nbytes argument?
>
> Not if you're volunteering to do the work. All I care about is that PA
> clients can crash for this reason, and I'd like that to stop happening.
Cool, I certainly volunteer to do the work. It should be trivial, so
I'll probably send a patch today.
> Note: gnome-control-
> currently checks the length rather than the NULL pointer, IIRC.
Mmm, I guess I should take this task too.
--
Tanu
StacktraceTop: acquire (b=<optimized out>) at pulsecore/ memblock. c:454 acquire (b=<optimized out>) at pulsecore/ memblock. c:453 0x7f6b582eaca0, data=data@ entry=0x7fff951 e3b48, length= length@ entry=0x7fff951 e3b38) at pulse/stream.c:1602 read_callback (s=0x7f6b582eaca0, length=4, userdata= 0x7f6b5823c020) at gvc-mixer- dialog. c:474 memblock_ callback (p=<optimized out>, channel=<optimized out>, offset=0, seek=PA_ SEEK_RELATIVE, chunk=0x7fff951 e3c00, userdata= 0x7f6b58200cb0) at pulse/context.c:365
pa_memblock_
pa_memblock_
pa_stream_peek (s=s@entry=
on_monitor_
pstream_