fix(clipboard): review fixes for the file lists kept per transfer

unix owner:
- A retired list closes its files and 8 MiB read buffers; they reopen,
  from offset 0, on the next read.
- The new list is built aside and swapped in only on success, so a failed
  rebuild leaves the current list in place.
- A rebuild with an unchanged id keeps its peers on the new list instead of
  retiring a duplicate, so re-copying a directory no longer pushes out a
  list that a transfer is still reading.

Windows:
- One served-list slot per list, recording every connection it was sent
  to, so pastes of the same list do not use up slots.
- A request for an earlier list is served before OleGetClipboard, so a busy
  clipboard cannot fail it.
- An IStream read over 16 MiB is split into requests the unix file server
  accepts. Reads up to 16 MiB take the unchanged path.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

libs/clipboard/src/platform/unix/serv_files.rs

@@ -5,9 +5,16 @@ use hbb_common::{
     log,
 };
 use parking_lot::Mutex;
-use std::{collections::VecDeque, path::PathBuf, sync::Arc, time::SystemTime, usize};
+use std::{
+    collections::VecDeque,
+    path::PathBuf,
+    sync::{atomic::Ordering, Arc},
+    time::SystemTime,
+    usize,
+};
 
-// Our clients request at most 4 MiB per range. A larger read is refused, not shortened:
+// Our clients request at most this much per range: macOS 4 MiB, FUSE less, and Windows
+// splits larger reads (WF_CLIPRDR_MAX_RANGE_READ). A larger read is refused, not shortened:
 // a short response reads as end of file to IStream callers on Windows.
 const MAX_RANGE_READ: u64 = 16 * 1024 * 1024;
 
@@ -119,6 +126,14 @@ impl ClipFiles {
         Ok(())
     }
 
+    // Close open files and their read buffers; `read_exact_at` reopens on the next read.
+    fn release_handles(&mut self) {
+        for file in self.file_list.iter_mut() {
+            file.offset.store(0, Ordering::Relaxed);
+            file.handle = None;
+        }
+    }
+
     fn get_files_for_audit(&self, request: &FileContentsRequest) -> Option<ClipboardFile> {
         if let FileContentsRequest::Range {
             file_idx, offset, ..
@@ -302,15 +317,21 @@ fn select_clip_files<'a>(
 }
 
 // Keep a list a peer was sent, so its streams do not read the new copy at the same indexes.
-fn retire_if_served(clip_files: &mut ClipFiles) {
-    if clip_files.served_to.is_empty() {
+fn retire_if_served(current: &mut ClipFiles, mut replaced: ClipFiles) {
+    if replaced.served_to.is_empty() {
         return;
     }
+    if replaced.id == current.id {
+        // Same descriptors: its peers read the new list at the same indexes.
+        current.served_to = replaced.served_to;
+        return;
+    }
+    replaced.release_handles();
     let mut retired = RETIRED_CLIP_FILES.lock();
     if retired.len() == MAX_RETIRED_CLIP_FILES {
         retired.pop_front();
     }
-    retired.push_back(std::mem::take(clip_files));
+    retired.push_back(replaced);
 }
 
 pub fn read_file_contents(
@@ -377,9 +398,13 @@ pub fn sync_files(files: &[String]) -> Result<(), CliprdrError> {
     {
         return Ok(());
     }
-    retire_if_served(&mut files_lock);
-    files_lock.sync_files(files, current)?;
-    files_lock.build_file_list_pdu()
+    // Build aside, so a failure leaves the current list in place.
+    let mut next = ClipFiles::default();
+    next.sync_files(files, current)?;
+    next.build_file_list_pdu()?;
+    let replaced = std::mem::replace(&mut *files_lock, next);
+    retire_if_served(&mut files_lock, replaced);
+    Ok(())
 }
 
 pub fn get_file_list_pdu(conn_id: i32) -> Vec<u8> {
@@ -680,4 +705,76 @@ mod sig_test {
 
         clear_files();
     }
+
+    #[test]
+    fn failed_recopy_keeps_the_current_list() {
+        let tmp = TmpDir::new("failed_recopy");
+        let first = tmp.join("first.bin");
+        fs::write(&first, b"AAAAAAAA").unwrap();
+        let missing = tmp.join("missing.bin");
+
+        let _guard = lock_clip_files();
+        clear_files();
+        sync_files(&[path_str(&first)]).unwrap();
+        let pdu = get_file_list_pdu(1);
+
+        assert!(sync_files(&[path_str(&missing)]).is_err());
+        assert_eq!(get_file_list_pdu(1), pdu);
+
+        clear_files();
+    }
+
+    #[test]
+    fn identical_recopy_keeps_its_peers_on_the_current_list() {
+        let tmp = TmpDir::new("same_recopy");
+        let dir = tmp.join("dir");
+        fs::create_dir_all(&dir).unwrap();
+        fs::write(dir.join("file.bin"), b"AAAAAAAA").unwrap();
+
+        let _guard = lock_clip_files();
+        clear_files();
+        sync_files(&[path_str(&dir)]).unwrap();
+        let id = file_list_id(&get_file_list_pdu(1));
+        let idx = CLIP_FILES.lock().first_file_index as i32;
+
+        // A selection with a directory is rebuilt on every copy; an unchanged rebuild must
+        // not use up the slots that lists still being read depend on.
+        for _ in 0..=MAX_RETIRED_CLIP_FILES {
+            sync_files(&[path_str(&dir)]).unwrap();
+        }
+        assert!(RETIRED_CLIP_FILES.lock().is_empty());
+        let read = |id| range_data(read_file_contents(1, 7, idx, 0x2, 0, 0, 4, id));
+        assert_eq!(read(Some(id)), b"AAAA");
+        assert_eq!(read(None), b"AAAA");
+
+        clear_files();
+    }
+
+    #[test]
+    fn retired_list_closes_its_files_and_reads_on() {
+        let tmp = TmpDir::new("retired_handles");
+        let first = tmp.join("first.bin");
+        fs::write(&first, b"AAAABBBB").unwrap();
+        let second = tmp.join("second.bin");
+        fs::write(&second, b"CCCCCCCC").unwrap();
+
+        let _guard = lock_clip_files();
+        clear_files();
+        sync_files(&[path_str(&first)]).unwrap();
+        let id = file_list_id(&get_file_list_pdu(1));
+        let idx = CLIP_FILES.lock().first_file_index as i32;
+        let read = |offset| range_data(read_file_contents(1, 7, idx, 0x2, offset, 0, 4, Some(id)));
+        // Leaves the file open at offset 4.
+        assert_eq!(read(0), b"AAAA");
+
+        sync_files(&[path_str(&second)]).unwrap();
+        assert!(RETIRED_CLIP_FILES.lock()[0]
+            .file_list
+            .iter()
+            .all(|file| file.handle.is_none()));
+        // The file reopens at its start, so continuing at offset 4 must seek.
+        assert_eq!(read(4), b"BBBB");
+
+        clear_files();
+    }
 }

libs/clipboard/src/windows/wf_cliprdr.c

@@ -57,6 +57,9 @@
 /* File lists kept after they were sent, so a transfer still in flight keeps reading its own
  * files after a later copy or paste replaces them. */
 #define WF_CLIPRDR_SERVED_FILE_LISTS 4u
+/* Largest range the unix file server serves in one request (MAX_RANGE_READ in
+ * serv_files.rs); larger IStream reads are split into requests of this size. */
+#define WF_CLIPRDR_MAX_RANGE_READ (16u * 1024u * 1024u)
 static const WCHAR WF_CLIPRDR_SUPERSCRIPT_DIGITS[] = L"\x00B9\x00B2\x00B3";
 static const WCHAR WF_CLIPRDR_INVALID_FILE_NAME_CHARS[] = L"<>:\"|?*";
 
@@ -398,8 +401,9 @@ typedef struct _CliprdrDataObject CliprdrDataObject;
 
 struct wf_served_file_list
 {
-	UINT32 connID;
 	UINT32 id;
+	UINT32 *connIDs; // connections the list was sent to
+	size_t nConnIDs;
 	size_t nFiles;
 	size_t first_file_index;
 	WCHAR **file_names;
@@ -498,6 +502,7 @@ static HRESULT CliprdrEnumFORMATETC_New(ULONG nFormats, FORMATETC *pFormatEtc,
 static void CliprdrEnumFORMATETC_Delete(CliprdrEnumFORMATETC *instance);
 
 static void CliprdrStream_Delete(CliprdrStream *instance);
+static HRESULT wf_cliprdr_stream_read_split(IStream *This, BYTE *pv, ULONG cb, ULONG *pcbRead);
 
 static BOOL try_open_clipboard(HWND hwnd)
 {
@@ -584,6 +589,9 @@ static HRESULT STDMETHODCALLTYPE CliprdrStream_Read(IStream *This, void *pv, ULO
 	if (instance->m_lOffset.QuadPart >= instance->m_lSize.QuadPart)
 		return S_FALSE;
 
+	if (cb > WF_CLIPRDR_MAX_RANGE_READ)
+		return wf_cliprdr_stream_read_split(This, (BYTE *)pv, cb, pcbRead);
+
 	ret = cliprdr_send_request_filecontents(clipboard, instance->m_connID, instance->m_streamId,
 											instance->m_clipDataId, instance->m_lIndex,
 											FILECONTENTS_RANGE, instance->m_lOffset.HighPart,
@@ -626,6 +634,26 @@ static HRESULT STDMETHODCALLTYPE CliprdrStream_Read(IStream *This, void *pv, ULO
 	return S_OK;
 }
 
+static HRESULT wf_cliprdr_stream_read_split(IStream *This, BYTE *pv, ULONG cb, ULONG *pcbRead)
+{
+	HRESULT hr = S_OK;
+	ULONG total = 0;
+	ULONG chunk;
+	ULONG n;
+
+	while (total < cb && hr == S_OK)
+	{
+		chunk = cb - total < WF_CLIPRDR_MAX_RANGE_READ ? cb - total : WF_CLIPRDR_MAX_RANGE_READ;
+		n = 0;
+		hr = CliprdrStream_Read(This, pv + total, chunk, &n);
+		total += n;
+	}
+	*pcbRead = total;
+	if (FAILED(hr))
+		return hr;
+	return total < cb ? S_FALSE : S_OK;
+}
+
 static HRESULT STDMETHODCALLTYPE CliprdrStream_Write(IStream *This, const void *pv, ULONG cb,
 													 ULONG *pcbWritten)
 {
@@ -2475,9 +2503,22 @@ static void wf_cliprdr_free_served_file_list(wfServedFileList *list)
 		free(list->file_names);
 	}
 	free(list->file_sizes);
+	free(list->connIDs);
 	ZeroMemory(list, sizeof(*list));
 }
 
+static BOOL wf_cliprdr_served_to(const wfServedFileList *list, UINT32 connID)
+{
+	size_t i;
+
+	for (i = 0; i < list->nConnIDs; i++)
+	{
+		if (list->connIDs[i] == connID)
+			return TRUE;
+	}
+	return FALSE;
+}
+
 static void wf_cliprdr_forget_served_file_lists(wfClipboard *clipboard)
 {
 	size_t i;
@@ -2487,46 +2528,67 @@ static void wf_cliprdr_forget_served_file_lists(wfClipboard *clipboard)
 	clipboard->next_served_file_list = 0;
 }
 
-static void wf_cliprdr_remember_served_file_list(wfClipboard *clipboard, UINT32 connID)
+static BOOL wf_cliprdr_copy_file_list(wfClipboard *clipboard, wfServedFileList *list)
 {
-	wfServedFileList *list;
 	size_t i;
 
-	for (i = 0; i < WF_CLIPRDR_SERVED_FILE_LISTS; i++)
-	{
-		list = &clipboard->served_file_lists[i];
-		if (list->file_names && list->connID == connID && list->id == clipboard->file_list_id)
-			return;
-	}
-
-	list = &clipboard->served_file_lists[clipboard->next_served_file_list];
-	clipboard->next_served_file_list =
-		(clipboard->next_served_file_list + 1) % WF_CLIPRDR_SERVED_FILE_LISTS;
-	wf_cliprdr_free_served_file_list(list);
-
 	list->file_names = (WCHAR **)calloc(clipboard->nFiles, sizeof(WCHAR *));
 	list->file_sizes = (UINT64 *)calloc(clipboard->nFiles, sizeof(UINT64));
 	if (!list->file_names || !list->file_sizes)
-	{
-		wf_cliprdr_free_served_file_list(list);
-		return;
-	}
+		return FALSE;
 	list->nFiles = clipboard->nFiles;
 	for (i = 0; i < clipboard->nFiles; i++)
 	{
 		if (!clipboard->file_names[i] ||
 			!(list->file_names[i] = _wcsdup(clipboard->file_names[i])))
-		{
-			wf_cliprdr_free_served_file_list(list);
-			return;
-		}
+			return FALSE;
 		if (clipboard->fileDescriptor[i])
 			list->file_sizes[i] = ((UINT64)clipboard->fileDescriptor[i]->nFileSizeHigh << 32) |
 								  clipboard->fileDescriptor[i]->nFileSizeLow;
 	}
-	list->connID = connID;
 	list->id = clipboard->file_list_id;
 	list->first_file_index = clipboard->first_file_index;
+	return TRUE;
+}
+
+/* One slot per list, shared by every connection it was sent to, so pastes of the same
+ * list do not push out a list another transfer is still reading. */
+static void wf_cliprdr_remember_served_file_list(wfClipboard *clipboard, UINT32 connID)
+{
+	wfServedFileList *list = NULL;
+	UINT32 *connIDs;
+	size_t i;
+
+	for (i = 0; i < WF_CLIPRDR_SERVED_FILE_LISTS; i++)
+	{
+		if (clipboard->served_file_lists[i].file_names &&
+			clipboard->served_file_lists[i].id == clipboard->file_list_id)
+		{
+			list = &clipboard->served_file_lists[i];
+			break;
+		}
+	}
+
+	if (!list)
+	{
+		list = &clipboard->served_file_lists[clipboard->next_served_file_list];
+		clipboard->next_served_file_list =
+			(clipboard->next_served_file_list + 1) % WF_CLIPRDR_SERVED_FILE_LISTS;
+		wf_cliprdr_free_served_file_list(list);
+		if (!wf_cliprdr_copy_file_list(clipboard, list))
+		{
+			wf_cliprdr_free_served_file_list(list);
+			return;
+		}
+	}
+
+	if (wf_cliprdr_served_to(list, connID))
+		return;
+	connIDs = (UINT32 *)realloc(list->connIDs, (list->nConnIDs + 1) * sizeof(UINT32));
+	if (!connIDs)
+		return;
+	connIDs[list->nConnIDs++] = connID;
+	list->connIDs = connIDs;
 }
 
 /* Serves a request whose stream came from an earlier file list sent to the same connection.
@@ -2541,8 +2603,8 @@ static BOOL wf_cliprdr_read_served_file_list(wfClipboard *clipboard,
 	for (i = 0; i < WF_CLIPRDR_SERVED_FILE_LISTS; i++)
 	{
 		if (clipboard->served_file_lists[i].file_names &&
-			clipboard->served_file_lists[i].connID == request->connID &&
-			clipboard->served_file_lists[i].id == request->clipDataId)
+			clipboard->served_file_lists[i].id == request->clipDataId &&
+			wf_cliprdr_served_to(&clipboard->served_file_lists[i], request->connID))
 		{
 			list = &clipboard->served_file_lists[i];
 			break;
@@ -3564,6 +3626,17 @@ wf_cliprdr_server_file_contents_request(CliprdrClientContext *context,
 		goto exit;
 	}
 
+	if (fileContentsRequest->haveClipDataId &&
+		fileContentsRequest->clipDataId != clipboard->file_list_id)
+	{
+		// Served by path from an earlier list; the clipboard is not needed.
+		ZeroMemory(&vStgMedium, sizeof(STGMEDIUM)); // read at exit
+		if (wf_cliprdr_read_served_file_list(clipboard, fileContentsRequest, pData, cbRequested,
+											 &uSize))
+			rc = CHANNEL_RC_OK;
+		goto exit;
+	}
+
 	hRet = OleGetClipboard(&pDataObj);
 
 	if (FAILED(hRet))
@@ -3581,15 +3654,6 @@ wf_cliprdr_server_file_contents_request(CliprdrClientContext *context,
 	vFormatEtc.lindex = fileContentsRequest->listIndex;
 	vFormatEtc.ptd = NULL;
 
-	if (fileContentsRequest->haveClipDataId &&
-		fileContentsRequest->clipDataId != clipboard->file_list_id)
-	{
-		if (wf_cliprdr_read_served_file_list(clipboard, fileContentsRequest, pData, cbRequested,
-											 &uSize))
-			rc = CHANNEL_RC_OK;
-		goto exit;
-	}
-
 	if (GetClipboardSequenceNumber() != clipboard->file_list_seq)
 	{
 		// The clipboard changed after the peer got its file list. The live FileContents