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.

Sign up for GitHub

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

Jump to bottom

shadow/rdpsnd: Fix race condition in rdpsnd channel server. #3357

Merged
merged 1 commit into from May 23, 2016
Merged

shadow/rdpsnd: Fix race condition in rdpsnd channel server. #3357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 48 additions & 10 deletions channels/rdpsnd/server/rdpsnd_main.c
View file
Open in desktop
Expand Up @@ -314,13 +314,16 @@ static UINT rdpsnd_server_select_format(RdpsndServerContext* context, int client
int bs;
int out_buffer_size;
AUDIO_FORMAT *format;
UINT error = CHANNEL_RC_OK;

if (client_format_index < 0 || client_format_index >= context->num_client_formats)
{
WLog_ERR(TAG, "index %d is not correct.", client_format_index);
return ERROR_INVALID_DATA;
}

EnterCriticalSection(&context->priv->lock);

context->priv->src_bytes_per_sample = context->src_format.wBitsPerSample / 8;
context->priv->src_bytes_per_frame = context->priv->src_bytes_per_sample * context->src_format.nChannels;

Expand All @@ -330,7 +333,8 @@ static UINT rdpsnd_server_select_format(RdpsndServerContext* context, int client
if (format->nSamplesPerSec == 0)
{
WLog_ERR(TAG, "invalid Client Sound Format!!");
return ERROR_INVALID_DATA;
error = ERROR_INVALID_DATA;
goto out;
}

switch(format->wFormatTag)
Expand Down Expand Up @@ -365,19 +369,24 @@ static UINT rdpsnd_server_select_format(RdpsndServerContext* context, int client
if (!newBuffer)
{
WLog_ERR(TAG, "realloc failed!");
return CHANNEL_RC_NO_MEMORY;
error = CHANNEL_RC_NO_MEMORY;
goto out;
}

context->priv->out_buffer = newBuffer;
context->priv->out_buffer_size = out_buffer_size;
}

freerdp_dsp_context_reset_adpcm(context->priv->dsp_context);
return CHANNEL_RC_OK;

out:
LeaveCriticalSection(&context->priv->lock);
return error;
}

/**
* Function description
* context->priv->lock should be obtained before calling this function
*
* @return 0 on success, otherwise a Win32 error code
*/
Expand Down Expand Up @@ -501,8 +510,15 @@ static UINT rdpsnd_server_send_samples(RdpsndServerContext* context, const void*
int cframesize;
UINT error = CHANNEL_RC_OK;

EnterCriticalSection(&context->priv->lock);

if (context->selected_client_format < 0)
return ERROR_INVALID_DATA;
{
/* It's possible while format negotiation has not been done */
WLog_WARN(TAG, "Drop samples because client format has not been negotiated.");
error = ERROR_NOT_READY;
goto out;
}

while (nframes > 0)
{
Expand All @@ -518,11 +534,15 @@ static UINT rdpsnd_server_send_samples(RdpsndServerContext* context, const void*
if (context->priv->out_pending_frames >= context->priv->out_frames)
{
if ((error = rdpsnd_server_send_audio_pdu(context, wTimestamp)))
{
WLog_ERR(TAG, "rdpsnd_server_send_audio_pdu failed with error %lu", error);

break;
}
}
}

out:
LeaveCriticalSection(&context->priv->lock);
return error;
}

Expand Down Expand Up @@ -568,18 +588,26 @@ static UINT rdpsnd_server_close(RdpsndServerContext* context)
wStream* s = context->priv->rdpsnd_pdu;
UINT error = CHANNEL_RC_OK;

if (context->selected_client_format < 0)
return ERROR_INVALID_DATA;
EnterCriticalSection(&context->priv->lock);

if (context->priv->out_pending_frames > 0)
{
if ((error = rdpsnd_server_send_audio_pdu(context, 0)))
if (context->selected_client_format < 0)
{
WLog_ERR(TAG, "Pending audio frame exists while no format selected.");
error = ERROR_INVALID_DATA;
}
else if ((error = rdpsnd_server_send_audio_pdu(context, 0)))
{
WLog_ERR(TAG, "rdpsnd_server_send_audio_pdu failed with error %lu", error);
return error;
}
}

LeaveCriticalSection(&context->priv->lock);

if (error)
return error;

context->selected_client_format = -1;

Stream_Write_UINT8(s, SNDC_CLOSE);
Expand Down Expand Up @@ -635,13 +663,19 @@ static UINT rdpsnd_server_start(RdpsndServerContext* context)
goto out_close;
}

if (!InitializeCriticalSectionEx(&context->priv->lock, 0, 0))
{
WLog_ERR(TAG, "InitializeCriticalSectionEx failed!");
goto out_pdu;
}

if (priv->ownThread)
{
context->priv->StopEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
if (!context->priv->StopEvent)
{
WLog_ERR(TAG, "CreateEvent failed!");
goto out_pdu;
goto out_lock;
}

context->priv->Thread = CreateThread(NULL, 0,
Expand All @@ -659,6 +693,8 @@ static UINT rdpsnd_server_start(RdpsndServerContext* context)
out_stopEvent:
CloseHandle(context->priv->StopEvent);
context->priv->StopEvent = NULL;
out_lock:
DeleteCriticalSection(&context->priv->lock);
out_pdu:
Stream_Free(context->priv->rdpsnd_pdu, TRUE);
context->priv->rdpsnd_pdu = NULL;
Expand Down Expand Up @@ -693,6 +729,8 @@ static UINT rdpsnd_server_stop(RdpsndServerContext* context)
}
}

DeleteCriticalSection(&context->priv->lock);

if (context->priv->rdpsnd_pdu)
Stream_Free(context->priv->rdpsnd_pdu, TRUE);

Expand Down
1 change: 1 addition & 0 deletions channels/rdpsnd/server/rdpsnd_main.h
View file
Open in desktop
Expand Up @@ -52,6 +52,7 @@ struct _rdpsnd_server_private
UINT32 src_bytes_per_sample;
UINT32 src_bytes_per_frame;
FREERDP_DSP_CONTEXT* dsp_context;
CRITICAL_SECTION lock; /* Protect out_buffer and related parameters */
};

#endif /* FREERDP_CHANNEL_SERVER_RDPSND_MAIN_H */
1 change: 0 additions & 1 deletion server/shadow/shadow_rdpsnd.c
View file
Open in desktop
Expand Up @@ -62,7 +62,6 @@ static void rdpsnd_activated(RdpsndServerContext* context)
}

context->SelectFormat(context, i);
context->SetVolume(context, 0x7FFF, 0x7FFF);
}
Copy link
Member

Choose a reason for hiding this comment

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

Any specific reason to remove this one? Does not look like it is related to the locking issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually there's no reason to have this line to set client volume in rdpsnd activated handler.
I copied the code from Windows/wf_rdpsnd.c when I introduce rdpsnd for shadow server last year without much considering. So I want to remove it this time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a subsystem implementation need to set volume (future), it should post SHADOW_MSG_OUT_AUDIO_OUT_VOLUME message to clients (shadow_client_post_msg/shadow_client_boardcast_msg)


int shadow_client_rdpsnd_init(rdpShadowClient* client)
Expand Down