Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ALSAAudioInput: use snd_pcm_drop() instead of snd_pcm_drain() in class destructor #3418

Merged
merged 1 commit into from Dec 15, 2018

Conversation

@davidebeatrici
Copy link
Member

commented Jun 9, 2018

snd_pcm_drain() waits for all pending frames to be played.

snd_pcm_drop() is better in our case, because we want the PCM stream to be stopped immediately.

This fixes a problem where Mumble would hang when closing the ALSA stream:

#0  0x00007ffff32484ec in futex_wait_cancelable (private=<optimized out>, expected=0, futex_word=0x555556d32c10) at ../sysdeps/unix/sysv/linux/futex-internal.h:88
#1  __pthread_cond_wait_common (abstime=0x0, mutex=0x555556d32bc0, cond=0x555556d32be8) at pthread_cond_wait.c:502
#2  __pthread_cond_wait (cond=cond@entry=0x555556d32be8, mutex=mutex@entry=0x555556d32bc0) at pthread_cond_wait.c:655
#3  0x00007ffff3509fbb in QWaitConditionPrivate::wait (time=18446744073709551615, this=0x555556d32bc0) at thread/qwaitcondition_unix.cpp:143
#4  QWaitCondition::wait (this=this@entry=0x555556beeaa0, mutex=mutex@entry=0x555556beea80, time=time@entry=18446744073709551615) at thread/qwaitcondition_unix.cpp:215
#5  0x00007ffff350872e in QThread::wait (this=<optimized out>, time=18446744073709551615) at thread/qthread_unix.cpp:741
#6  0x0000555555750bc5 in ALSAAudioInput::~ALSAAudioInput (this=0x555556c67c00, __in_chrg=<optimized out>) at ALSAAudio.cpp:279
#7  ALSAAudioInput::~ALSAAudioInput (this=0x555556c67c00, __in_chrg=<optimized out>) at ALSAAudio.cpp:280
#8  0x000055555568c10e in boost::detail::sp_counted_base::release (this=0x555556d325c0) at /usr/include/boost/smart_ptr/detail/sp_counted_base_gcc_x86.hpp:146
#9  boost::detail::shared_count::~shared_count (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/boost/smart_ptr/detail/shared_count.hpp:473
#10 boost::shared_ptr<AudioInput>::~shared_ptr (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/boost/smart_ptr/shared_ptr.hpp:336
#11 boost::shared_ptr<AudioInput>::reset (this=<synthetic pointer>) at /usr/include/boost/smart_ptr/shared_ptr.hpp:667
#12 Audio::stop () at Audio.cpp:296
#13 0x0000555555692eeb in ConfigDialog::apply (this=this@entry=0x555556c2aca0) at ConfigDialog.cpp:179
#14 0x0000555555694a2e in ConfigDialog::accept (this=0x555556c2aca0) at ConfigDialog.cpp:199
#15 0x00007ffff371f195 in QMetaObject::activate (sender=sender@entry=0x555556bfa1e0, signalOffset=<optimized out>, local_signal_index=local_signal_index@entry=1, argv=argv@entry=0x0)
    at kernel/qobject.cpp:3767
#16 0x00007ffff371f867 in QMetaObject::activate (sender=sender@entry=0x555556bfa1e0, m=m@entry=0x7ffff55c14c0 <QDialogButtonBox::staticMetaObject>, 
    local_signal_index=local_signal_index@entry=1, argv=argv@entry=0x0) at kernel/qobject.cpp:3629
#17 0x00007ffff50805c3 in QDialogButtonBox::accepted (this=this@entry=0x555556bfa1e0) at .moc/moc_qdialogbuttonbox.cpp:281
#18 0x00007ffff5080be0 in QDialogButtonBoxPrivate::_q_handleButtonClicked (this=<optimized out>) at widgets/qdialogbuttonbox.cpp:864
#19 0x00007ffff371f195 in QMetaObject::activate (sender=sender@entry=0x555556b30400, signalOffset=<optimized out>, local_signal_index=local_signal_index@entry=2, 
    argv=argv@entry=0x7fffffffc070) at kernel/qobject.cpp:3767
#20 0x00007ffff371f867 in QMetaObject::activate (sender=sender@entry=0x555556b30400, m=m@entry=0x7ffff55b83a0 <QAbstractButton::staticMetaObject>, 
    local_signal_index=local_signal_index@entry=2, argv=argv@entry=0x7fffffffc070) at kernel/qobject.cpp:3629
#21 0x00007ffff4fddb72 in QAbstractButton::clicked (this=this@entry=0x555556b30400, _t1=<optimized out>) at .moc/moc_qabstractbutton.cpp:308
#22 0x00007ffff4fddd8a in QAbstractButtonPrivate::emitClicked (this=0x555556ed3320) at widgets/qabstractbutton.cpp:414
#23 0x00007ffff4fdf16a in QAbstractButtonPrivate::click (this=0x555556ed3320) at widgets/qabstractbutton.cpp:407
#24 0x00007ffff4fdf35d in QAbstractButton::mouseReleaseEvent (this=0x555556b30400, e=0x7fffffffc520) at widgets/qabstractbutton.cpp:1011
#25 0x00007ffff4f2b378 in QWidget::event (this=0x555556b30400, event=0x7fffffffc520) at kernel/qwidget.cpp:9277
#26 0x00007ffff4eec6cc in QApplicationPrivate::notify_helper (this=this@entry=0x5555560b8410, receiver=receiver@entry=0x555556b30400, e=e@entry=0x7fffffffc520)
    at kernel/qapplication.cpp:3732
#27 0x00007ffff4ef43df in QApplication::notify (this=<optimized out>, receiver=0x555556b30400, e=0x7fffffffc520) at kernel/qapplication.cpp:3208
#28 0x00007ffff36ef938 in QCoreApplication::notifyInternal2 (receiver=receiver@entry=0x555556b30400, event=event@entry=0x7fffffffc520) at kernel/qcoreapplication.cpp:1050
#29 0x00007ffff4ef33b2 in QCoreApplication::sendEvent (event=<optimized out>, receiver=<optimized out>) at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:234
#30 QApplicationPrivate::sendMouseEvent (receiver=receiver@entry=0x555556b30400, event=event@entry=0x7fffffffc520, alienWidget=alienWidget@entry=0x555556b30400, 
    nativeWidget=0x555556bfa1e0, buttonDown=buttonDown@entry=0x7ffff55e1810 <qt_button_down>, lastMouseReceiver=..., spontaneous=true) at kernel/qapplication.cpp:2711
#31 0x00007ffff4f464d3 in QWidgetWindow::handleMouseEvent (this=this@entry=0x555556780a50, event=event@entry=0x7fffffffc920) at kernel/qwidgetwindow.cpp:661
#32 0x00007ffff4f48b09 in QWidgetWindow::event (this=0x555556780a50, event=0x7fffffffc920) at kernel/qwidgetwindow.cpp:280
#33 0x00007ffff4eec6cc in QApplicationPrivate::notify_helper (this=this@entry=0x5555560b8410, receiver=receiver@entry=0x555556780a50, e=e@entry=0x7fffffffc920)
    at kernel/qapplication.cpp:3732
#34 0x00007ffff4ef3e84 in QApplication::notify (this=0x7fffffffdaf0, receiver=0x555556780a50, e=0x7fffffffc920) at kernel/qapplication.cpp:3491
#35 0x00007ffff36ef938 in QCoreApplication::notifyInternal2 (receiver=receiver@entry=0x555556780a50, event=event@entry=0x7fffffffc920) at kernel/qcoreapplication.cpp:1050
#36 0x00007ffff46fb603 in QCoreApplication::sendSpontaneousEvent (event=0x7fffffffc920, receiver=0x555556780a50)
    at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:237
#37 QGuiApplicationPrivate::processMouseEvent (e=0x5555587e9500) at kernel/qguiapplication.cpp:1960
#38 0x00007ffff46fd0d5 in QGuiApplicationPrivate::processWindowSystemEvent (e=e@entry=0x5555587e9500) at kernel/qguiapplication.cpp:1741
#39 0x00007ffff46d503b in QWindowSystemInterface::sendWindowSystemEvents (flags=...) at kernel/qwindowsysteminterface.cpp:984
#40 0x00007fffea3b72cb in QPAEventDispatcherGlib::processEvents (this=0x555556109ee0, flags=...) at qeventdispatcher_glib.cpp:70
#41 0x00007ffff36edb6a in QEventLoop::exec (this=this@entry=0x7fffffffcb90, flags=..., flags@entry=...) at kernel/qeventloop.cpp:212
#42 0x00007ffff50d98b7 in QDialog::exec (this=this@entry=0x555556c2aca0) at dialogs/qdialog.cpp:546
#43 0x0000555555626931 in MainWindow::on_qaConfigDialog_triggered (this=0x5555563ddec0) at MainWindow.cpp:2374
#44 0x0000555555766861 in MainWindow::qt_static_metacall (_o=_o@entry=0x5555563ddec0, _c=_c@entry=QMetaObject::InvokeMetaMethod, _id=_id@entry=51, _a=_a@entry=0x7fffffffcdd0)
    at ../../release/.moc/mumble/moc_MainWindow.cpp:539
#45 0x0000555555767055 in MainWindow::qt_metacall (this=0x5555563ddec0, _c=QMetaObject::InvokeMetaMethod, _id=51, _a=0x7fffffffcdd0)
    at ../../release/.moc/mumble/moc_MainWindow.cpp:651
#46 0x00007ffff371f259 in QMetaObject::activate (sender=sender@entry=0x55555677e330, signalOffset=<optimized out>, local_signal_index=local_signal_index@entry=1, 
    argv=argv@entry=0x7fffffffcdd0) at kernel/qobject.cpp:3782
#47 0x00007ffff371f867 in QMetaObject::activate (sender=sender@entry=0x55555677e330, m=m@entry=0x7ffff55b2840 <QAction::staticMetaObject>, 
    local_signal_index=local_signal_index@entry=1, argv=argv@entry=0x7fffffffcdd0) at kernel/qobject.cpp:3629
#48 0x00007ffff4ee5e52 in QAction::triggered (this=this@entry=0x55555677e330, _t1=<optimized out>) at .moc/moc_qaction.cpp:376
#49 0x00007ffff4ee859c in QAction::activate (this=0x55555677e330, event=<optimized out>) at kernel/qaction.cpp:1167
#50 0x00007ffff4fdf0fb in QAbstractButtonPrivate::click (this=0x555556b58390) at widgets/qabstractbutton.cpp:397
#51 0x00007ffff4fdf35d in QAbstractButton::mouseReleaseEvent (this=0x555556b57330, e=0x7fffffffd2b0) at widgets/qabstractbutton.cpp:1011
#52 0x00007ffff50c04ba in QToolButton::mouseReleaseEvent (this=<optimized out>, e=<optimized out>) at widgets/qtoolbutton.cpp:620
#53 0x00007ffff4f2b378 in QWidget::event (this=0x555556b57330, event=0x7fffffffd2b0) at kernel/qwidget.cpp:9277
#54 0x00007ffff4fe052b in QAbstractButton::event (this=this@entry=0x555556b57330, e=e@entry=0x7fffffffd2b0) at widgets/qabstractbutton.cpp:968
#55 0x00007ffff50c0554 in QToolButton::event (this=0x555556b57330, event=0x7fffffffd2b0) at widgets/qtoolbutton.cpp:983
#56 0x00007ffff4eec6cc in QApplicationPrivate::notify_helper (this=this@entry=0x5555560b8410, receiver=receiver@entry=0x555556b57330, e=e@entry=0x7fffffffd2b0)
    at kernel/qapplication.cpp:3732
#57 0x00007ffff4ef43df in QApplication::notify (this=<optimized out>, receiver=0x555556b57330, e=0x7fffffffd2b0) at kernel/qapplication.cpp:3208
#58 0x00007ffff36ef938 in QCoreApplication::notifyInternal2 (receiver=receiver@entry=0x555556b57330, event=event@entry=0x7fffffffd2b0) at kernel/qcoreapplication.cpp:1050
#59 0x00007ffff4ef33b2 in QCoreApplication::sendEvent (event=<optimized out>, receiver=<optimized out>) at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:234
#60 QApplicationPrivate::sendMouseEvent (receiver=receiver@entry=0x555556b57330, event=event@entry=0x7fffffffd2b0, alienWidget=alienWidget@entry=0x555556b57330, 
    nativeWidget=0x555556b49f30, buttonDown=buttonDown@entry=0x7ffff55e1810 <qt_button_down>, lastMouseReceiver=..., spontaneous=true) at kernel/qapplication.cpp:2711
#61 0x00007ffff4f464d3 in QWidgetWindow::handleMouseEvent (this=this@entry=0x555556c9f900, event=event@entry=0x7fffffffd6b0) at kernel/qwidgetwindow.cpp:661
#62 0x00007ffff4f48b09 in QWidgetWindow::event (this=0x555556c9f900, event=0x7fffffffd6b0) at kernel/qwidgetwindow.cpp:280
#63 0x00007ffff4eec6cc in QApplicationPrivate::notify_helper (this=this@entry=0x5555560b8410, receiver=receiver@entry=0x555556c9f900, e=e@entry=0x7fffffffd6b0)
    at kernel/qapplication.cpp:3732
#64 0x00007ffff4ef3e84 in QApplication::notify (this=0x7fffffffdaf0, receiver=0x555556c9f900, e=0x7fffffffd6b0) at kernel/qapplication.cpp:3491
#65 0x00007ffff36ef938 in QCoreApplication::notifyInternal2 (receiver=receiver@entry=0x555556c9f900, event=event@entry=0x7fffffffd6b0) at kernel/qcoreapplication.cpp:1050
#66 0x00007ffff46fb603 in QCoreApplication::sendSpontaneousEvent (event=0x7fffffffd6b0, receiver=0x555556c9f900)
    at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:237
#67 QGuiApplicationPrivate::processMouseEvent (e=0x555556cc2b10) at kernel/qguiapplication.cpp:1960
#68 0x00007ffff46fd0d5 in QGuiApplicationPrivate::processWindowSystemEvent (e=e@entry=0x555556cc2b10) at kernel/qguiapplication.cpp:1741
#69 0x00007ffff46d503b in QWindowSystemInterface::sendWindowSystemEvents (flags=...) at kernel/qwindowsysteminterface.cpp:984
#70 0x00007fffea3b72cb in QPAEventDispatcherGlib::processEvents (this=0x555556109ee0, flags=...) at qeventdispatcher_glib.cpp:70
#71 0x00007ffff36edb6a in QEventLoop::exec (this=this@entry=0x7fffffffd920, flags=..., flags@entry=...) at kernel/qeventloop.cpp:212
#72 0x00007ffff36f6ed4 in QCoreApplication::exec () at kernel/qcoreapplication.cpp:1338
#73 0x00005555555c2e3b in main (argc=<optimized out>, argv=<optimized out>) at main.cpp:549
ALSAAudioInput: use snd_pcm_drop() instead of snd_pcm_drain() in clas…
…s destructor

snd_pcm_drain() waits for all pending frames to be played.

snd_pcm_drop() is better in our case, because we want the PCM stream to be stopped immediately.

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:alsa branch from f855573 to fbbdf2e Jun 16, 2018

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

When would it hang, and for how long? As long as the playback buffer is big? When closing mumble and switching away from the alsa engine?

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

My first thought is finishing playing the intermediate buffered audio is good. ALSAAudio runs in a thread, so why is this an issue. Is it because when we switch sound engines we wait for the old one to clean up before starting the new one?

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

From the API I wonder if a close would implicitly drop as well.

Closes the specified PCM handle and frees all associated resources.

@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented Jun 18, 2018

When would it hang, and for how long?

I have to terminate it forcefully.

As long as the playback buffer is big?

No, Mumble freezes only when the stream is stopped (via snd_pcm_drain()).

When closing mumble and switching away from the alsa engine?

Also when applying the settings with ALSA, because the stream is terminated and then started again.

My first thought is finishing playing the intermediate buffered audio is good. ALSAAudio runs in a thread, so why is this an issue. Is it because when we switch sound engines we wait for the old one to clean up before starting the new one?

I think it's an issue within the ALSA library, because it started happening recently.

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

From the API I linked, the close description links three example. None use drop or drain before close. (Maybe this is because they play a stream to end? Although one does call a loop function.)

Can you try close without drop or drain?

I also wondered if just cutting of an audio stream could produce audio artefacts. But I guess not?

Then this should be fine. If close is not enough, we can use drop.

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

As long as the playback buffer is big?

No, Mumble freezes only when the stream is stopped (via snd_pcm_drain()).

What I meant was does it lock for x time if the buffer is x time "long".

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

If it hangs forever my guess is we use a stream and that never "ends" and hence drain never returns?

You say it only started happening recently, probably from outside of our control/not our lib/implementation? Driver changed how they handle drain on streams maybe?

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

If drain indeed blocks, there is a drain call further up the file in an error case. Should be replaced with drop then as well. L336

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

This is about ALSAAudioInput. In this file there is also ALSAAudioOutput. That uses drain as well. Why is that not a problem?

For the normal case that actually just uses close. Drain is called in some specific branch cases. Maybe we just don't run into it yet?

@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented Jun 18, 2018

I think so.

@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented Jul 1, 2018

@Kissaki What do you think about replacing snd_pcm_drain() with snd_pcm_drop() everywhere?

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jul 6, 2018

Can we test the ALSAAudioOutput case and verify when we run into the branch it is also in issue there?

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jul 6, 2018

If close also drains I would prefer to just and only call close. We need confirmation on that though. As I said the docs are not very specific about this.

Also, the other questions I raised could help us figure out what is going on.

@thefloweringash thefloweringash referenced this pull request Oct 21, 2018
5 of 9 tasks complete
@timokau

This comment has been minimized.

Copy link

commented Oct 21, 2018

Just so you know, there are nix users that experience this issue (NixOS/nixpkgs#48004) and we are pulling in this patch to fix this now (NixOS/nixpkgs#48774).

@davidebeatrici davidebeatrici merged commit d7ef517 into mumble-voip:master Dec 15, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.