From ec28567362e1a0f30cb20df42f66f6f6b221c95b Mon Sep 17 00:00:00 2001 From: fufesou <13586388+fufesou@users.noreply.github.com> Date: Tue, 3 Sep 2024 20:55:45 +0800 Subject: [PATCH] fix: win, file clipboard (#9243) 1. Return the result of `wait_response_event()` in `cliprdr_send_format_list()` 2. Add recv flags to avoid waiting a long time. Signed-off-by: fufesou --- libs/clipboard/src/lib.rs | 19 +++++--- libs/clipboard/src/platform/windows.rs | 63 +++++++++++++++++-------- libs/clipboard/src/windows/wf_cliprdr.c | 41 +++++++++++----- src/client/io_loop.rs | 1 + 4 files changed, 86 insertions(+), 38 deletions(-) diff --git a/libs/clipboard/src/lib.rs b/libs/clipboard/src/lib.rs index 1a9a04757..a5da25512 100644 --- a/libs/clipboard/src/lib.rs +++ b/libs/clipboard/src/lib.rs @@ -5,7 +5,7 @@ use std::{ }; #[cfg(any(target_os = "windows", feature = "unix-file-copy-paste",))] -use hbb_common::{allow_err, log}; +use hbb_common::{allow_err, bail}; use hbb_common::{ lazy_static, tokio::sync::{ @@ -25,6 +25,8 @@ pub use context_send::*; const ERR_CODE_SERVER_FUNCTION_NONE: u32 = 0x00000001; #[cfg(target_os = "windows")] const ERR_CODE_INVALID_PARAMETER: u32 = 0x00000002; +#[cfg(target_os = "windows")] +const ERR_CODE_SEND_MSG: u32 = 0x00000003; pub(crate) use platform::create_cliprdr_context; @@ -198,7 +200,7 @@ pub fn get_rx_cliprdr_server(conn_id: i32) -> Arc ResultType<()> { #[cfg(target_os = "windows")] return send_data_to_channel(conn_id, data); #[cfg(not(target_os = "windows"))] @@ -210,25 +212,28 @@ fn send_data(conn_id: i32, data: ClipboardFile) { } #[cfg(any(target_os = "windows", feature = "unix-file-copy-paste",))] #[inline] -fn send_data_to_channel(conn_id: i32, data: ClipboardFile) { - // no need to handle result here +fn send_data_to_channel(conn_id: i32, data: ClipboardFile) -> ResultType<()> { if let Some(msg_channel) = VEC_MSG_CHANNEL .read() .unwrap() .iter() .find(|x| x.conn_id == conn_id) { - allow_err!(msg_channel.sender.send(data)); + msg_channel.sender.send(data)?; + Ok(()) + } else { + bail!("conn_id not found"); } } #[cfg(feature = "unix-file-copy-paste")] #[inline] -fn send_data_to_all(data: ClipboardFile) { - // no need to handle result here +fn send_data_to_all(data: ClipboardFile) -> ResultType<()> { + // Need more tests to see if it's necessary to handle the error. for msg_channel in VEC_MSG_CHANNEL.read().unwrap().iter() { allow_err!(msg_channel.sender.send(data.clone())); } + Ok(()) } #[cfg(test)] diff --git a/libs/clipboard/src/platform/windows.rs b/libs/clipboard/src/platform/windows.rs index 8fc917c6f..5d1aa086d 100644 --- a/libs/clipboard/src/platform/windows.rs +++ b/libs/clipboard/src/platform/windows.rs @@ -7,7 +7,7 @@ use crate::{ allow_err, send_data, ClipboardFile, CliprdrError, CliprdrServiceContext, ResultType, - ERR_CODE_INVALID_PARAMETER, ERR_CODE_SERVER_FUNCTION_NONE, VEC_MSG_CHANNEL, + ERR_CODE_INVALID_PARAMETER, ERR_CODE_SEND_MSG, ERR_CODE_SERVER_FUNCTION_NONE, VEC_MSG_CHANNEL, }; use hbb_common::log; use std::{ @@ -998,7 +998,7 @@ extern "C" fn notify_callback(conn_id: UINT32, msg: *const NOTIFICATION_MESSAGE) } }; // no need to handle result here - send_data(conn_id as _, data); + allow_err!(send_data(conn_id as _, data)); 0 } @@ -1045,7 +1045,13 @@ extern "C" fn client_format_list( .iter() .for_each(|msg_channel| allow_err!(msg_channel.sender.send(data.clone()))); } else { - send_data(conn_id, data); + match send_data(conn_id, data) { + Ok(_) => {} + Err(e) => { + log::error!("failed to send format list: {:?}", e); + return ERR_CODE_SEND_MSG; + } + } } 0 @@ -1067,9 +1073,13 @@ extern "C" fn client_format_list_response( msg_flags ); let data = ClipboardFile::FormatListResponse { msg_flags }; - send_data(conn_id, data); - - 0 + match send_data(conn_id, data) { + Ok(_) => 0, + Err(e) => { + log::error!("failed to send format list response: {:?}", e); + ERR_CODE_SEND_MSG + } + } } extern "C" fn client_format_data_request( @@ -1090,10 +1100,13 @@ extern "C" fn client_format_data_request( conn_id, requested_format_id ); - // no need to handle result here - send_data(conn_id, data); - - 0 + match send_data(conn_id, data) { + Ok(_) => 0, + Err(e) => { + log::error!("failed to send format data request: {:?}", e); + ERR_CODE_SEND_MSG + } + } } extern "C" fn client_format_data_response( @@ -1125,9 +1138,13 @@ extern "C" fn client_format_data_response( msg_flags, format_data, }; - send_data(conn_id, data); - - 0 + match send_data(conn_id, data) { + Ok(_) => 0, + Err(e) => { + log::error!("failed to send format data response: {:?}", e); + ERR_CODE_SEND_MSG + } + } } extern "C" fn client_file_contents_request( @@ -1175,9 +1192,13 @@ extern "C" fn client_file_contents_request( clip_data_id, }; log::debug!("client_file_contents_request called, data: {:?}", &data); - send_data(conn_id, data); - - 0 + match send_data(conn_id, data) { + Ok(_) => 0, + Err(e) => { + log::error!("failed to send file contents request: {:?}", e); + ERR_CODE_SEND_MSG + } + } } extern "C" fn client_file_contents_response( @@ -1213,7 +1234,11 @@ extern "C" fn client_file_contents_response( msg_flags, stream_id ); - send_data(conn_id, data); - - 0 + match send_data(conn_id, data) { + Ok(_) => 0, + Err(e) => { + log::error!("failed to send file contents response: {:?}", e); + ERR_CODE_SEND_MSG + } + } } diff --git a/libs/clipboard/src/windows/wf_cliprdr.c b/libs/clipboard/src/windows/wf_cliprdr.c index c8f2038a1..5bca54052 100644 --- a/libs/clipboard/src/windows/wf_cliprdr.c +++ b/libs/clipboard/src/windows/wf_cliprdr.c @@ -220,7 +220,8 @@ struct wf_clipboard HWND hwnd; HANDLE hmem; HANDLE thread; - HANDLE response_data_event; + HANDLE formatDataRespEvent; + BOOL formatDataRespReceived; LPDATAOBJECT data_obj; HANDLE data_obj_mutex; @@ -228,6 +229,7 @@ struct wf_clipboard ULONG req_fsize; char *req_fdata; HANDLE req_fevent; + BOOL req_f_received; size_t nFiles; size_t file_array_size; @@ -1444,7 +1446,7 @@ static UINT cliprdr_send_format_list(wfClipboard *clipboard, UINT32 connID) return rc; } -UINT wait_response_event(UINT32 connID, wfClipboard *clipboard, HANDLE event, void **data) +UINT wait_response_event(UINT32 connID, wfClipboard *clipboard, HANDLE event, BOOL* recvedFlag, void **data) { UINT rc = ERROR_SUCCESS; clipboard->context->IsStopped = FALSE; @@ -1456,7 +1458,14 @@ UINT wait_response_event(UINT32 connID, wfClipboard *clipboard, HANDLE event, vo DWORD waitRes = WaitForSingleObject(event, waitOnceTimeoutMillis); if (waitRes == WAIT_TIMEOUT && clipboard->context->IsStopped == FALSE) { - continue; + if ((*recvedFlag) == TRUE) { + // The data has been received, but the event is still not signaled. + // We just skip the rest of the waiting and reset the flag. + *recvedFlag = FALSE; + } else { + // The data has not been received yet, we should continue to wait. + continue; + } } if (clipboard->context->IsStopped == TRUE) @@ -1524,13 +1533,14 @@ static UINT cliprdr_send_data_request(UINT32 connID, wfClipboard *clipboard, UIN formatDataRequest.connID = connID; formatDataRequest.requestedFormatId = remoteFormatId; clipboard->requestedFormatId = formatId; + clipboard->formatDataRespReceived = FALSE; rc = clipboard->context->ClientFormatDataRequest(clipboard->context, &formatDataRequest); if (rc != ERROR_SUCCESS) { return rc; } - wait_response_event(connID, clipboard, clipboard->response_data_event, &clipboard->hmem); + return wait_response_event(connID, clipboard, clipboard->formatDataRespEvent, &clipboard->formatDataRespReceived, &clipboard->hmem); } UINT cliprdr_send_request_filecontents(wfClipboard *clipboard, UINT32 connID, const void *streamid, ULONG index, @@ -1552,13 +1562,14 @@ UINT cliprdr_send_request_filecontents(wfClipboard *clipboard, UINT32 connID, co fileContentsRequest.cbRequested = nreq; fileContentsRequest.clipDataId = 0; fileContentsRequest.msgFlags = 0; + clipboard->req_f_received = FALSE; rc = clipboard->context->ClientFileContentsRequest(clipboard->context, &fileContentsRequest); if (rc != ERROR_SUCCESS) { return rc; } - return wait_response_event(connID, clipboard, clipboard->req_fevent, (void **)&clipboard->req_fdata); + return wait_response_event(connID, clipboard, clipboard->req_fevent, &clipboard->req_f_received, (void **)&clipboard->req_fdata); } static UINT cliprdr_send_response_filecontents( @@ -2545,7 +2556,7 @@ exit: response.requestedFormatData = (BYTE *)buff; if (ERROR_SUCCESS != clipboard->context->ClientFormatDataResponse(clipboard->context, &response)) { - // CAUTION: if failed to send, server will wait a long time + // CAUTION: if failed to send, server will wait a long time, default 30 seconds. } if (buff) @@ -2621,9 +2632,11 @@ wf_cliprdr_server_format_data_response(CliprdrClientContext *context, rc = CHANNEL_RC_OK; } while (0); - if (!SetEvent(clipboard->response_data_event)) + if (!SetEvent(clipboard->formatDataRespEvent)) { - // CAUTION: critical error here, process will hang up until wait timeout default 3min. + // If failed to set event, set flag to indicate the event is received. + DEBUG_CLIPRDR("wf_cliprdr_server_format_data_response(), SetEvent failed with 0x%x", GetLastError()); + clipboard->formatDataRespReceived = TRUE; rc = ERROR_INTERNAL_ERROR; } return rc; @@ -2899,7 +2912,9 @@ wf_cliprdr_server_file_contents_response(CliprdrClientContext *context, if (!SetEvent(clipboard->req_fevent)) { - // CAUTION: critical error here, process will hang up until wait timeout default 3min. + // If failed to set event, set flag to indicate the event is received. + DEBUG_CLIPRDR("wf_cliprdr_server_file_contents_response(), SetEvent failed with 0x%x", GetLastError()); + clipboard->req_f_received = TRUE; } return rc; } @@ -2934,14 +2949,16 @@ BOOL wf_cliprdr_init(wfClipboard *clipboard, CliprdrClientContext *cliprdr) (formatMapping *)calloc(clipboard->map_capacity, sizeof(formatMapping)))) goto error; - if (!(clipboard->response_data_event = CreateEvent(NULL, TRUE, FALSE, NULL))) + if (!(clipboard->formatDataRespEvent = CreateEvent(NULL, TRUE, FALSE, NULL))) goto error; + clipboard->formatDataRespReceived = FALSE; if (!(clipboard->data_obj_mutex = CreateMutex(NULL, FALSE, "data_obj_mutex"))) goto error; if (!(clipboard->req_fevent = CreateEvent(NULL, TRUE, FALSE, NULL))) goto error; + clipboard->req_f_received = FALSE; if (!(clipboard->thread = CreateThread(NULL, 0, cliprdr_thread_func, clipboard, 0, NULL))) goto error; @@ -3002,8 +3019,8 @@ BOOL wf_cliprdr_uninit(wfClipboard *clipboard, CliprdrClientContext *cliprdr) clipboard->data_obj = NULL; } - if (clipboard->response_data_event) - CloseHandle(clipboard->response_data_event); + if (clipboard->formatDataRespEvent) + CloseHandle(clipboard->formatDataRespEvent); if (clipboard->data_obj_mutex) CloseHandle(clipboard->data_obj_mutex); diff --git a/src/client/io_loop.rs b/src/client/io_loop.rs index b222e4118..1a209ca0a 100644 --- a/src/client/io_loop.rs +++ b/src/client/io_loop.rs @@ -353,6 +353,7 @@ impl Remote { } else { if let Err(e) = ContextSend::make_sure_enabled() { log::error!("failed to restart clipboard context: {}", e); + // to-do: Show msgbox with "Don't show again" option }; log::debug!("Send system clipboard message to remote"); let msg = crate::clipboard_file::clip_2_msg(clip);