shadow/rdpsnd: Fix race condition in rdpsnd channel server. #3357
Conversation
|
Refer to this link for build results (access rights to CI server needed): |
| { | ||
| WLog_ERR(TAG, "rdpsnd_server_send_audio_pdu failed with error %lu", error); | ||
| return error; |
There was a problem hiding this comment.
I think error should be set here since it is initialized with CHANNEL_RC_OK.
There was a problem hiding this comment.
It is actually correct. Here error = rdpsnd_server_send_audio_pdu(context, 0). It fail only when error is not CHANNEL_RC_OK
|
@realjiangms except my comment +1. Thanks. |
…t buffer and format parameters are not protected. This cause some data inconsistence if rdpsnd has own thread
realjiangms
force-pushed
the
fix_rdpsnd_race
branch
from
c06cbae
to
444b678
Compare
|
Refer to this link for build results (access rights to CI server needed): |
| @@ -62,7 +62,6 @@ static void rdpsnd_activated(RdpsndServerContext* context) | |||
| } | |||
|
|
|||
| context->SelectFormat(context, i); | |||
| context->SetVolume(context, 0x7FFF, 0x7FFF); | |||
There was a problem hiding this comment.
Any specific reason to remove this one? Does not look like it is related to the locking issue.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
|
@realjiangms Except my remark +1 |
The output buffer and format parameters are not protected. This cause some data inconsistency if rdpsnd has own thread.
A typical race scenario is:
context->selected_client_format = client_format_index; // Now context->priv->out_buffer is still 0
It will pass the check of "if (context->selected_client_format < 0)", but will crash when try to copy samples to context->priv->out_buffer