FreeRDP / FreeRDP-old

DEPRECATED VERSION - Check https://github.com/FreeRDP/FreeRDP : FreeRDP is a free remote desktop protocol client
http://www.freerdp.com
Apache License 2.0
80 stars 882 forks source link

Audio input segfault #26

Open otavio opened 13 years ago

otavio commented 13 years ago

While testing audio input plugin, on an embedded system, we got a segfault when we close the open device. In case you open the audio recorder and close it after using it, it segfaults.

otavio commented 13 years ago

@llyzs I think you might have an idea about what the issue is. Can you take a look?

llyzs commented 13 years ago

is there a backtrace you can provide?

otavio commented 13 years ago

I can try to gather something ...

otavio commented 13 years ago
...

dvcman_write_channel
drdynvc_write_data
set_variable_uint
audin_send_open_reply_pdu
dvcman_write_channel
drdynvc_write_data
set_variable_uint
OpenEvent
OpenEvent
audin_alsa_thread_func
audin_alsa_thread_func: in
audin_alsa_set_params
audin_alsa_set_params: actual rate 22050 / channel 2 is different from target rate 22050 / channel 1, resampling required.
audin_alsa_thread_receive
audin_dsp_resample
audin_alsa_thread_receive: resampled 1524 frames at 22050 to 1524 frames at 22050
Segmentation fault

Does it enlight something?

eduardobeloni commented 13 years ago

I've also got the following:

...

audin_dsp_encode_ima_adpcm_sample
audin_dsp_encode_ima_adpcm_sample
audin_dsp_encode_ima_adpcm_sample
audin_dsp_encode_ima_adpcm_sample
audin_dsp_encode_ima_adpcm_sample
audin_receive_wave_data
audin_send_incoming_data_pdu
dvcman_write_channel
drdynvc_write_data
set_variable_uint
dvcman_write_channel
drdynvc_write_data
set_variable_uint
OpenEvent
OpenEvent
OpenEvent
OpenEventProcessReceived
signal_data_in
thread_process_data_in
thread_process_message
process_CLOSE_REQUEST_PDU
get_variable_uint
dvcman_close_channel
dvcman_find_channel_by_id
dvcman_close_channel: channel 0 closed
dvcman_close_channel_iface
audin_on_close
audin_alsa_close
audin_alsa_thread_receive
audin_dsp_resample

It seems like there are data being processed after the device has been closed...

otavio commented 13 years ago

It seems it is a sort of memory corruption. It is being a pain in the ass to figure the root cause of it since it only happens on the embedded machine (probably due the memory restriction).

The segfault happens in many different places.

Someone has any idea how to debug it?

llyzs commented 13 years ago

Please try this patch:

diff --git a/channels/drdynvc/audin/alsa/audin_alsa.c b/channels/drdynvc/audin/alsa/audin_alsa.c
index 1869ea7..60e4174 100644
--- a/channels/drdynvc/audin/alsa/audin_alsa.c
+++ b/channels/drdynvc/audin/alsa/audin_alsa.c
@@ -126,6 +126,9 @@ audin_alsa_thread_receive(struct alsa_device_data * alsa_data,

    while (frames > 0)
    {
+       if (wait_obj_is_set(alsa_data->term_event))
+           break;
+
        cframes = alsa_data->frames_per_packet - alsa_data->buffer_frames;
        if (cframes > frames)
            cframes = frames;
@@ -148,7 +151,14 @@ audin_alsa_thread_receive(struct alsa_device_data * alsa_data,
                encoded_size = alsa_data->buffer_frames * tbytes_per_frame;
            }

-           ret = alsa_data->receive_func(encoded_data, encoded_size, alsa_data->user_data);
+           if (wait_obj_is_set(alsa_data->term_event))
+           {
+               ret = 1;
+               frames = 0;
+           }
+           else
+               ret = alsa_data->receive_func(encoded_data, encoded_size, alsa_data->user_data);
+
            alsa_data->buffer_frames = 0;
            if (encoded_data != alsa_data->buffer)
                free(encoded_data);
eduardobeloni commented 13 years ago

The following is the output after applying your patch. This happen when I open Sound Recorder, then close it (without even clicking Start Recording):

...

OpenEvent
OpenEvent: event 11
audin_alsa_thread_receive
audin_dsp_resample
audin_alsa_thread_receive: resampled 1524 frames at 22050 to 1524 frames at 22050
OpenEvent
OpenEvent: event 10
OpenEventProcessReceived
OpenEventProcessReceived: receive openHandle 1 dataLength 2 totalLength 2 dataFlags 3
signal_data_in
thread_process_data_in
thread_process_message
thread_process_message: data_size 2 cmd 0x4
0000 40 00                                           @.
process_CLOSE_REQUEST_PDU
get_variable_uint
process_CLOSE_REQUEST_PDU: ChannelId=0
dvcman_close_channel: channel 0 closed
dvcman_channel_close: id=0
audin_on_close
audin_on_close:
audin_alsa_close
audin_alsa_close:
audin_alsa_thread_receive
audin_dsp_resample
audin_alsa_thread_receive: resampled 1524 frames at 22050 to 1524 frames at 22050
llyzs commented 13 years ago

Please try this one, but if it still does not fix it, we need to capture exactly which line caused the issue by inserting a lot of printf's line-by-line.

diff --git a/channels/drdynvc/dvcman.c b/channels/drdynvc/dvcman.c
index 2eb0ebd..0c17aa1 100644
--- a/channels/drdynvc/dvcman.c
+++ b/channels/drdynvc/dvcman.c
@@ -402,14 +402,14 @@ dvcman_close_channel(IWTSVirtualChannelManager * pChannelMgr, uint32 ChannelId)
        LLOGLN(0, ("dvcman_close_channel: ChannelId %d not found!", ChannelId));
        return 1;
    }
+   LLOGLN(0, ("dvcman_close_channel: channel %d closed", ChannelId));
+   ichannel = (IWTSVirtualChannel *) channel;
+   ichannel->Close(ichannel);
    if (channel->dvc_data)
    {
        free(channel->dvc_data);
        channel->dvc_data = NULL;
    }
-   LLOGLN(0, ("dvcman_close_channel: channel %d closed", ChannelId));
-   ichannel = (IWTSVirtualChannel *) channel;
-   ichannel->Close(ichannel);
    return 0;
 }
atong commented 13 years ago

(move this to FreeRdp/issues?)

Against 1.0, I appended tid %x to audin_alsa_thread_func's debugging, the ERROR is when alsa->buffer == NULL

Here is a subset of the debug output, # are my notes

DBG_DVC drdynvc_process_create_request (208): ChannelId=5 ChannelName=AUDIO_INPUT DBG_DVC drdynvc_process_create_request (219): channel created DBG_DVC audin_alsa_open (300): DBG_DVC audin_alsa_thread_func (172): in tid b3dfc700 # original thread DBG_DVC audin_alsa_close (312): # original should be closing DBG_DVC audin_alsa_open (300): # but, new thread starts DBG_DVC audin_alsa_thread_func (172): in tid b27f4700 DBG_DVC audin_alsa_thread_func (218): out tid b27f4700 # this gets original's close request, cleans up it self when it should be running ERROR tid b3dfc700 # original runs into alsa->buffer == NULL

otavio commented 13 years ago

Please move it to FreeRDP/issue as we do need to track it down for 1.0 release. 0.9 is dead code for us now.