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

Critical interlocking bug in ASYNC AsyncAudioReader #639

Closed
sa2blv opened this issue Sep 2, 2023 · 1 comment
Closed

Critical interlocking bug in ASYNC AsyncAudioReader #639

sa2blv opened this issue Sep 2, 2023 · 1 comment

Comments

@sa2blv
Copy link

sa2blv commented Sep 2, 2023

I have spent at least a couple of years to find a solution to this bug
This problem occurs when using multiple threads in SvxLink, together with SIP Logic.
SvxLink and SIP Logic do access a shared memory space. When thread 1 accesses to read data, the memory allocated by the buf parameter, the other tread (2) Is trying to write to the same memory space
This will cause a Segmentation fault in ram.
I found out, that memcpy, used to access the shared memory space, isn’t thread-safe.
I managed to solve the problem by adding yield “mutex” to the kernel, to stop the other threads to access the same memory space, while memcpy is operating.

Ex of solution

AsyncAudioReader.h

`
#include // std::mutex

private:
AudioReader(const AudioReader&);
AudioReader& operator=(const AudioReader&);

float *buf;
int   buf_size;
bool  input_stopped;
int   samples_in_buf;
std::mutex mtx;           // mutex for critical section

`
AsyncAudioReader.cpp

`int AudioReader::writeSamples(const float *samples, int count)

int samples_to_read = 0;

try
{

mtx.lock();

if (buf != 0 && samples != NULL)
{
 //  std::cout << "before " << count << "\r\n";


  samples_to_read = min(count, buf_size - samples_in_buf);
  memcpy(buf + samples_in_buf, samples, samples_to_read * sizeof(*buf));


  samples_in_buf += samples_to_read;
}
else
{
   if( samples == NULL)
     {
     std::cout << "AudioReader::data incorreft null" ;
   }



}



input_stopped = (samples_to_read == 0);
mtx.unlock();

}
catch (int myNum)
{
std::cout << "AudioReader::writeSamples fault" ;
return 0;
}

Original code

AsyncAudioReader.cpp
`int AudioReader::readSamples(float *samples, int count)
{
assert(count > 0);

buf = samples;
buf_size = count;
samples_in_buf = 0;
if (input_stopped)
{
input_stopped = false;
sourceResumeOutput();
}
buf = 0;
buf_size = 0;

//printf("AudioReader::readSamples: samples_in_buf=%d\n", samples_in_buf);

return samples_in_buf;

} /* AudioReader::readSamples */

int AudioReader::writeSamples(const float *samples, int count)
{
int samples_to_read = 0;
if (buf != 0)
{
samples_to_read = min(count, buf_size - samples_in_buf);
memcpy(buf + samples_in_buf, samples, samples_to_read * sizeof(*buf));
samples_in_buf += samples_to_read;
}

input_stopped = (samples_to_read == 0);

return samples_to_read;

} /* AudioReader::writeSamples */
`

GDB out when fault occurred

Thread 11 (Thread 0x7ffff5a13700 (LWP 1121643)):
#0 __memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:538
#1 0x00007ffff7f78fd9 in Async::AudioReader::writeSamples(float const*, int) () from /lib/x86_64-linux-gnu/libasyncaudio.so.1.6
#2 0x00007ffff7f7841d in Async::AudioFifo::writeSamplesFromFifo() () from /lib/x86_64-linux-gnu/libasyncaudio.so.1.6
#3 0x00007ffff7f790b1 in Async::AudioReader::readSamples(float*, int) () from /lib/x86_64-linux-gnu/libasyncaudio.so.1.6
#4 0x000055555566d911 in SipLogic::mediaPortGetFrame (this=0x5555559eaa30, port=0x555555a4cbc8, frame=0x7ffff5a12df0) at /home/fura/svxlink/svxlink/src/svxlink/svxlink/SipLogic.cpp:1086
#5 0x00005555557cac31 in get_frame ()
#6 0x00005555557d0413 in clock_callback ()
#7 0x00005555557f131b in clock_thread ()
#8 0x000055555584f004 in thread_main ()
#9 0x00007ffff7c70609 in start_thread (arg=) at pthread_create.c:477
#10 0x00007ffff7607133 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 4 (Thread 0x7ffff6214700 (LWP 1113341)):
#0 0x00007ffff75fcfcb in __GI___select (nfds=1024, readfds=0x7ffff62137a8, writefds=0x7ffff62139c8, exceptfds=0x7ffff6213be8, timeout=0x7ffff6213660) at ../sysdeps/unix/sysv/linux/select.c:41
#1 0x000055555585147a in pj_sock_select ()
#2 0x000055555584d7f6 in pj_ioqueue_poll ()
#3 0x00005555557a66de in pjsip_endpt_handle_events2 ()
#4 0x000055555576d8e2 in pjsua_handle_events ()
#5 0x000055555576d93c in worker_thread ()
#6 0x000055555584f004 in thread_main ()
#7 0x00007ffff7c70609 in start_thread (arg=) at pthread_create.c:477
#8 0x00007ffff7607133 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 3 (Thread 0x7ffff6a15700 (LWP 1113340)):
#0 futex_abstimed_wait_cancelable (private=0, abstime=0x0, clockid=0, expected=0, futex_word=0x555555a21d80) at ../sysdeps/nptl/futex-internal.h:320
#1 do_futex_wait (sem=sem@entry=0x555555a21d80, abstime=0x0, clockid=0) at sem_waitcommon.c:112
#2 0x00007ffff7c7a548 in __new_sem_wait_slow (sem=0x555555a21d80, abstime=0x0, clockid=0) at sem_waitcommon.c:184
#3 0x000055555584f7dc in pj_sem_wait ()
#4 0x00005555557cf0ff in event_worker_thread ()
#5 0x000055555584f004 in thread_main ()
#6 0x00007ffff7c70609 in start_thread (arg=) at pthread_create.c:477
#7 0x00007ffff7607133 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 2 (Thread 0x7ffff7216700 (LWP 1113339)):
#0 0x00007ffff75fcfcb in __GI___select (nfds=1024, readfds=0x7ffff7215808, writefds=0x7ffff7215a28, exceptfds=0x7ffff7215c48, timeout=0x7ffff72156c0) at ../sysdeps/unix/sysv/linux/select.c:41
#1 0x000055555585147a in pj_sock_select ()
#2 0x000055555584d7f6 in pj_ioqueue_poll ()
#3 0x00005555557cd530 in worker_proc ()
#4 0x000055555584f004 in thread_main ()
#5 0x00007ffff7c70609 in start_thread (arg=) at pthread_create.c:477
#6 0x00007ffff7607133 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Thread 1 (Thread 0x7ffff741d740 (LWP 1113335)):
#0 0x00007ffff75fd0e6 in __pselect (nfds=12, readfds=0x7fffffffdd60, writefds=0x7fffffffdde0, exceptfds=0x0, timeout=, sigmask=) at ../sysdeps/unix/sysv/linux/pselect.c:48
#1 0x00007ffff7fae897 in Async::CppApplication::exec() () from /lib/x86_64-linux-gnu/libasynccpp.so.1.6
#2 0x0000555555620c8e in main (argc=, argv=) at /home/fura/svxlink/svxlink/src/svxlink/svxlink/svxlink.cpp:533

AsyncAudioReader.zip

@sm0svx
Copy link
Owner

sm0svx commented Sep 3, 2023

The Async library is not written to be threadsafe so it can only be used safely from one thread. Threads should only be used in special cases where the asynchronous, or event based, mechanism is not possible to use. Such valid cases include: thirdparty libraries that only support a blocking API paradigm, muticore requirements, latency requirements.

The standard way to solve threaded use cases in SvxLink is to put the minimum amount of code that need to be in its own thread and then use a threadsafe mechanism to communicate data between the threads. An approach using a threadsafe FIFO to read samples from an RTL dongle can be seen in RtlUsb.cpp.

I'll close this issue since it's not a bug in Async but rather an API violation.

@sm0svx sm0svx closed this as completed Sep 3, 2023
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

2 participants