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

Unhandled asynchronous SIGIO #394

Closed
jansenmd opened this issue Apr 23, 2024 · 5 comments
Closed

Unhandled asynchronous SIGIO #394

jansenmd opened this issue Apr 23, 2024 · 5 comments

Comments

@jansenmd
Copy link

When using libasound.so on embedded systems, I've come across several instances where an unhandled SIGIO is detected. This can occur when closing a hardware microphone device that was set up to use asynchronous calls backs via snd_async_add_pcm_handler.

Example: when opening a hardware device with asynchronous callbacks:

  • Sets up a signal handler for SIGIO via sigaction.
  • Enable generation of signals from the fd by setting the O_ASYNC flag: fcntl(fd, F_SETFL, flags | O_ASYNC)
  • Sets the signal to be generated: fcntl(fd, F_SETSIG, (long)SIGIO)

Now, when data is available to be read from the fd, SIGIO should be generated and handled by the signal handler set in sigaction. That appears to work fine.

The current teardown process looks like this:

  • Clear/reset the signal handler via sigaction.
  • Disable generation of signals from the fd by clearing the O_ASYNC flag: fcntl(fd, F_SETFL, flags & ~O_ASYNC)
  • Close the fd.

This teardown process seems to set up a race condition where an unhandled SIGIO can be generated between unregistering the signal handler and actually stopping any more signals from occurring. Since the default action for SIGIO is termination, this can cause an application to unexpectedly terminate.

@perexg perexg closed this as completed in 5600b90 Jun 4, 2024
@perexg
Copy link
Member

perexg commented Jun 4, 2024

Thanks. The above commit should move sigaction after fcntl.

@velik76
Copy link

velik76 commented Sep 5, 2024

Hi all,

We are using alsa 1.2.12 with patch mentioned above, but have same behavior as reported by jansenmd, via debugging we got that the part inside of:

switch (handler->type) {
in the snd_async_del_handler() function

will be never called.

perexg added a commit that referenced this issue Sep 6, 2024
…ng sigaction as last")

A wrong list head is used to check if the given list with async handlers
is empty. Correct this.

Link: #394
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
@perexg
Copy link
Member

perexg commented Sep 6, 2024

@velik76 : It's right. Could you test 3b9f3b9 ?

@velik76
Copy link

velik76 commented Sep 6, 2024

@perexg:

  1. For last patch via some additional debug messages I can acknowledge, that code of snd_async_del_handler() now works as expected

  2. Improvement idea for the snd_pcm_close(): it will be may be better to call the snd_pcm_drop() after the async handler deactivated. Because between snd_pcm_drop() and snd_async_del_handler() the signal could be generated that is sometimes could be not Ok

perexg added a commit that referenced this issue Sep 6, 2024
It reduces probablity to activate the async handler when snd_pcm_close() is called.

Link: #394
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
@perexg
Copy link
Member

perexg commented Sep 6, 2024

@velik76 : I made that change in 93d7645 . But the async handler can be called before signal deactivation. It cannot be avoided and the application must handle this correctly in the async handler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants