Skip to content
This repository has been archived by the owner on Mar 7, 2024. It is now read-only.

[mod_google_transcribe] [Crash-Fix] Crash in google_speech_session_cleanup function #92

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

saurabh-iitrke
Copy link

@saurabh-iitrke saurabh-iitrke commented Sep 13, 2022

Use native switch_resample_destroy function instead of speex_resampler_destroy directly for better error handling

This fix is for the crash with bt output.

gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x0000705e5905b535 in __GI_abort () at abort.c:79
#2  0x0000705e590b2518 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x705e591bc28d "%s\n") at ../sysdeps/posix/libc_fatal.c:181
#3  0x0000705e590b8c3a in malloc_printerr (str=str@entry=0x705e591bdfe0 "double free or corruption (out)") at malloc.c:5359
#4  0x0000705e590ba770 in _int_free (av=0x705e591f3c40 <main_arena>, p=0x705d641e6790, have_lock=<optimized out>) at malloc.c:4321
#5  0x0000705e589ef226 in speex_resampler_destroy () from /usr/lib/x86_64-linux-gnu/libspeexdsp.so.1
#6  0x0000705e5546cb3d in google_speech_session_cleanup (session=session@entry=0x705e30420b68, channelIsClosing=channelIsClosing@entry=1) at google_glue.cpp:460
#7  0x0000705e5546af7e in capture_callback (bug=0x705d640ba088, user_data=0x705d5c0a3360, type=SWITCH_ABC_TYPE_CLOSE) at mod_google_transcribe.c:65
#8  0x0000705e598cf2d9 in switch_core_media_bug_close (bug=bug@entry=0x705d3b7c1858, destroy=destroy@entry=SWITCH_FALSE) at src/switch_core_media_bug.c:1303
#9  0x0000705e598cf510 in switch_core_media_bug_remove_all_function (session=session@entry=0x705e30420b68, function=function@entry=0x0) at src/switch_core_media_bug.c:1271
#10 0x0000705e598ecc4a in switch_core_session_hangup_state (session=session@entry=0x705e30420b68, force=force@entry=SWITCH_TRUE) at src/switch_core_state_machine.c:835
#11 0x0000705e598ee49d in switch_core_session_run (session=0x705e30420b68) at src/switch_core_state_machine.c:612
#12 0x0000705e598e8c0e in switch_core_session_thread (thread=<optimized out>, obj=0x705e30420b68) at src/switch_core_session.c:1726
#13 0x0000705e598e42eb in switch_core_session_thread_pool_worker (thread=0x705d5819aa60, obj=<optimized out>) at src/switch_core_session.c:1790
#14 0x0000705e59e4352c in dummy_worker (opaque=0x705d5819aa60) at threadproc/unix/thread.c:151
#15 0x0000705e5957ffa3 in start_thread (arg=<optimized out>) at pthread_create.c:486
#16 0x0000705e59131eff in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

@saurabh-iitrke saurabh-iitrke changed the title Use switch_resample_destroy function instead of speex_resampler_destr… [mod_google_transcribe] [Crash-Fix] Use switch_resample_destroy function instead of speex_resampler_destr… Sep 13, 2022
@saurabh-iitrke saurabh-iitrke changed the title [mod_google_transcribe] [Crash-Fix] Use switch_resample_destroy function instead of speex_resampler_destr… [mod_google_transcribe] [Crash-Fix] Crash in google_speech_session_cleanup function Sep 13, 2022
Copy link
Collaborator

@davehorton davehorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't compile

make[4]: Entering directory '/usr/local/src/freeswitch/src/mod/applications/mod_google_transcribe'
  CXX      mod_google_transcribe_la-google_glue.lo
google_glue.cpp: In function ‘switch_status_t google_speech_session_cleanup(switch_core_session_t*, int, switch_media_bug_t*)’:
google_glue.cpp:622:35: error: cannot convert ‘SpeexResamplerState**’ {aka ‘SpeexResamplerState_**’} to ‘switch_audio_resampler_t**’
           switch_resample_destroy(&cb->resampler);

@davehorton
Copy link
Collaborator

also doesn't really make sense. The allocation of the resampler is done with the 'speex' interface.

cb->resampler = speex_resampler_init(1, sampleRate, 8000, SWITCH_RESAMPLE_QUALITY, &err);

@saurabh-iitrke
Copy link
Author

Hi @davehorton , yeah. the fix given is not correct. I realised that.
I have added a commit to fix the race condition that is leading to crash with double free or corruption (out).
Also, in order to not touch existing media bug events, i have added a custom event in src/include/switch_types.h

-	SMBF_FIRST = (1 << 26)
+	SMBF_FIRST = (1 << 26),
+	SMBF_STATUS_CHECK = (1 << 27)

@davehorton
Copy link
Collaborator

Hmm, it seems undesirable to have to add code to the freeswitch core in order to fix this race condition. Can you give me more background on what this race condition actually is? I am unclear about the problem we are trying to fix here

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

Successfully merging this pull request may close these issues.

2 participants