diff --git a/idd/LGCommon/CClipboardChannel.cpp b/idd/LGCommon/CClipboardChannel.cpp index bcf0995b..3e25429e 100644 --- a/idd/LGCommon/CClipboardChannel.cpp +++ b/idd/LGCommon/CClipboardChannel.cpp @@ -24,13 +24,13 @@ #include "CDebug.h" #include +#include #include +#include #include namespace { - static constexpr DWORD WAIT_FIRST_OBJECT_VALUE = 0; - struct ClipboardCallbackScope { CClipboardChannel * channel; @@ -171,15 +171,35 @@ bool CClipboardChannel::Attach(HANDLE mapping, uint64_t epoch, } m_stop = CreateEventW(nullptr, TRUE, FALSE, nullptr); - m_kick = CreateEventW(nullptr, FALSE, FALSE, nullptr); - if (!m_stop || !m_kick) + if (!m_stop) { - DEBUG_ERROR_HR(GetLastError(), - "Failed to create clipboard channel events"); - if (m_kick) - CloseHandle(m_kick); - if (m_stop) - CloseHandle(m_stop); + const DWORD error = GetLastError(); + DEBUG_ERROR_HR(error, + "Failed to create clipboard channel stop event"); + UnmapViewOfFile(view); + CloseHandle(mapping); + return false; + } + m_kick = CreateEventW(nullptr, FALSE, FALSE, nullptr); + if (!m_kick) + { + const DWORD error = GetLastError(); + DEBUG_ERROR_HR(error, + "Failed to create clipboard channel kick event"); + CloseHandle(m_stop); + m_stop = nullptr; + UnmapViewOfFile(view); + CloseHandle(mapping); + return false; + } + m_writeReady = CreateEventW(nullptr, FALSE, FALSE, nullptr); + if (!m_writeReady) + { + const DWORD error = GetLastError(); + DEBUG_ERROR_HR(error, + "Failed to create clipboard channel credit event"); + CloseHandle(m_kick); + CloseHandle(m_stop); m_kick = nullptr; m_stop = nullptr; UnmapViewOfFile(view); @@ -212,10 +232,12 @@ bool CClipboardChannel::Attach(HANDLE mapping, uint64_t epoch, m_threadId = 0; m_view = nullptr; m_mapping = nullptr; + CloseHandle(m_writeReady); CloseHandle(m_kick); CloseHandle(m_stop); UnmapViewOfFile(view); CloseHandle(mapping); + m_writeReady = nullptr; m_kick = nullptr; m_stop = nullptr; return false; @@ -259,6 +281,8 @@ void CClipboardChannel::Detach() CSRWExclusiveLock lock(m_lifecycleLock); if (m_thread) CloseHandle(m_thread); + if (m_writeReady) + CloseHandle(m_writeReady); if (m_kick) CloseHandle(m_kick); if (m_stop) @@ -270,6 +294,7 @@ void CClipboardChannel::Detach() m_thread = nullptr; m_threadId = 0; + m_writeReady = nullptr; m_kick = nullptr; m_stop = nullptr; m_in = nullptr; @@ -287,8 +312,21 @@ void CClipboardChannel::Detach() void CClipboardChannel::Kick(uint64_t epoch) { CSRWSharedLock lock(m_lifecycleLock); - if (Available() && epoch == m_epoch && m_kick) - SetEvent(m_kick); + if (Available() && epoch == m_epoch && m_kick && m_writeReady) + { + if (!SetEvent(m_kick)) + { + const DWORD error = GetLastError(); + DEBUG_ERROR_HR(error, + "Failed to signal clipboard channel receive event"); + } + if (!SetEvent(m_writeReady)) + { + const DWORD error = GetLastError(); + DEBUG_ERROR_HR(error, + "Failed to signal clipboard channel credit event"); + } + } } void CClipboardChannel::Reset(uint64_t epoch, uint32_t reason) @@ -306,34 +344,85 @@ void CClipboardChannel::Reset(uint64_t epoch, uint32_t reason) Fail(instance, reason, false); } +bool CClipboardChannel::WaitWritable(HANDLE stop, DWORD timeout) +{ + if (!stop || stop == INVALID_HANDLE_VALUE) + return false; + + CSRWSharedLock lock(m_lifecycleLock); + if (!Available() || !m_writeReady) + return false; + const HANDLE events[] = { stop, m_writeReady }; + const DWORD result = WaitForMultipleObjects( + ARRAYSIZE(events), events, FALSE, timeout); + if (result == WAIT_OBJECT_0) + return false; + if (result == WAIT_OBJECT_0 + 1 || result == WAIT_TIMEOUT) + return true; + if (result == WAIT_FAILED) + DEBUG_ERROR_HR(GetLastError(), + "Failed to wait for clipboard channel credit"); + else + DEBUG_ERROR("Invalid clipboard channel credit wait result: %lu", + static_cast(result)); + return false; +} + ClipboardChannelResult CClipboardChannel::Send( const KVMFRClipboardMessage& record, const void * data) { - if (!ValidRecord(record, data != nullptr)) + const ClipboardChannelWrite write = { record, data }; + size_t accepted = 0; + return SendBatch(&write, 1, accepted); +} + +ClipboardChannelResult CClipboardChannel::SendBatch( + const ClipboardChannelWrite * records, size_t count, size_t& accepted) +{ + accepted = 0; + if (!records || !count || count > KVMFR_CLIPBOARD_SLOT_COUNT) return ClipboardChannelResult::FAILED; + for (size_t i = 0; i < count; ++i) + if (!ValidRecord(records[i].record, records[i].data != nullptr)) + return ClipboardChannelResult::FAILED; CSRWSharedLock lifecycleLock(m_lifecycleLock); if (!Available() || !m_out || !m_doorbell) return ClipboardChannelResult::FAILED; + bool failed = false; { CSRWExclusiveLock writeLock(m_writeLock); - uint32_t ticket; - ClipboardRingSlot * slot = CClipboardRing::BeginWrite(*m_out, ticket); - if (!slot) - return ClipboardChannelResult::BUSY; + while (accepted < count) + { + uint32_t ticket; + ClipboardRingSlot * slot = CClipboardRing::BeginWrite(*m_out, ticket); + if (!slot) + break; - slot->header = record; - if (record.length) - memcpy(slot->data, data, record.length); - if (!CClipboardRing::EndWrite(*m_out, ticket)) - return ClipboardChannelResult::FAILED; + const ClipboardChannelWrite& write = records[accepted]; + slot->header = write.record; + if (write.record.length) + memcpy(slot->data, write.data, write.record.length); + if (!CClipboardRing::EndWrite(*m_out, ticket)) + { + failed = true; + break; + } + ++accepted; + } } - // Once the producer index advances the record belongs to the channel. - // Doorbells are only a latency optimization; the peer also polls. - m_doorbell->ClipboardKick(m_epoch); - return ClipboardChannelResult::ACCEPTED; + if (accepted) + { + // Once the producer index advances the records belong to the channel. + // Doorbells are only a latency optimization; the peer also polls. + m_doorbell->ClipboardKick(m_epoch); + } + if (failed) + return ClipboardChannelResult::FAILED; + return accepted == count ? ClipboardChannelResult::ACCEPTED : + ClipboardChannelResult::BUSY; } void CClipboardChannel::SetHandler(IClipboardChannelHandler * handler) @@ -381,6 +470,23 @@ void CClipboardChannel::ClearHandler(IClipboardChannelHandler * handler) CClipboardChannel::DrainResult CClipboardChannel::Drain() { + struct CreditNotifier + { + bool pending = false; + IClipboardChannelDoorbell * doorbell = nullptr; + uint64_t epoch = 0; + + void Notify() + { + if (pending && doorbell) + doorbell->ClipboardKick(epoch); + pending = false; + } + + ~CreditNotifier() { Notify(); } + } credit; + size_t consumed = 0; + for (;;) { KVMFRClipboardMessage record = {}; @@ -417,21 +523,25 @@ CClipboardChannel::DrainResult CClipboardChannel::Drain() ClipboardChannelResult result = ValidRecord(record, record.length != 0) ? - PublishRecord(record, data.empty() ? nullptr : data.data()) : + PublishRecord(record, std::move(data)) : ClipboardChannelResult::FAILED; if (result == ClipboardChannelResult::BUSY) return DrainResult::BUSY; - IClipboardChannelDoorbell * doorbell; - uint64_t epoch; { CSRWSharedLock lifecycleLock(m_lifecycleLock); if (!Available() || !m_in || !m_doorbell) return DrainResult::STOPPED; if (!CClipboardRing::EndRead(*m_in, ticket)) return DrainResult::CORRUPT; - doorbell = m_doorbell; - epoch = m_epoch; + credit.pending = true; + credit.doorbell = m_doorbell; + credit.epoch = m_epoch; + } + if (++consumed == KVMFR_CLIPBOARD_SLOT_COUNT) + { + credit.Notify(); + consumed = 0; } if (result == ClipboardChannelResult::FAILED) @@ -441,8 +551,6 @@ CClipboardChannel::DrainResult CClipboardChannel::Drain() // local callback and peer reset outside the ring/lifecycle locks. return DrainResult::CORRUPT; } - else - doorbell->ClipboardKick(epoch); } } @@ -508,6 +616,8 @@ void CClipboardChannel::CleanupDeferredDetach() if (m_kick) CloseHandle(m_kick); + if (m_writeReady) + CloseHandle(m_writeReady); if (m_stop) CloseHandle(m_stop); if (m_view) @@ -517,6 +627,7 @@ void CClipboardChannel::CleanupDeferredDetach() // A later external Detach closes the now-signaled thread handle. m_threadId = 0; + m_writeReady = nullptr; m_kick = nullptr; m_stop = nullptr; m_in = nullptr; @@ -546,10 +657,11 @@ void CClipboardChannel::PublishState(bool available, uint64_t epoch) } ClipboardChannelResult CClipboardChannel::PublishRecord( - const KVMFRClipboardMessage& record, const uint8_t * data) + const KVMFRClipboardMessage& record, std::vector&& data) { if (InClipboardCallback(this)) - return m_handler ? m_handler->ClipboardRecord(record, data) : + return m_handler ? + m_handler->ClipboardRecord(record, std::move(data)) : ClipboardChannelResult::BUSY; CSRWExclusiveLock lock(m_handlerLock); @@ -557,7 +669,7 @@ ClipboardChannelResult CClipboardChannel::PublishRecord( return ClipboardChannelResult::BUSY; CClipboardCallbackScope scope(this); - return m_handler->ClipboardRecord(record, data); + return m_handler->ClipboardRecord(record, std::move(data)); } void CClipboardChannel::PublishReset(uint64_t epoch, uint32_t reason) diff --git a/idd/LGCommon/CClipboardChannel.h b/idd/LGCommon/CClipboardChannel.h index f3f009b1..1d5afcd0 100644 --- a/idd/LGCommon/CClipboardChannel.h +++ b/idd/LGCommon/CClipboardChannel.h @@ -27,7 +27,9 @@ #include #include +#include #include +#include enum class ClipboardChannelResult { @@ -36,16 +38,24 @@ enum class ClipboardChannelResult FAILED, }; +struct ClipboardChannelWrite +{ + KVMFRClipboardMessage record = {}; + const void * data = nullptr; +}; + class IClipboardChannelHandler { public: virtual ~IClipboardChannelHandler() = default; virtual void ClipboardState(bool available, uint64_t epoch) = 0; - // data is borrowed and remains valid only for the duration of this call. - // BUSY leaves the shared ring slot occupied so it can be retried later. + // data owns record.length bytes and may be moved by the handler. BUSY + // leaves the shared ring slot occupied so it can be reconstructed and + // retried later. virtual ClipboardChannelResult ClipboardRecord( - const KVMFRClipboardMessage& record, const uint8_t * data) = 0; + const KVMFRClipboardMessage& record, + std::vector&& data) = 0; virtual void ClipboardReset(uint64_t epoch, uint32_t reason) = 0; }; @@ -75,16 +85,17 @@ private: CSRWLock m_handlerLock; CSRWLock m_writeLock; - HANDLE m_mapping = nullptr; - ClipboardMapping * m_view = nullptr; - ClipboardRing * m_in = nullptr; - ClipboardRing * m_out = nullptr; - HANDLE m_stop = nullptr; - HANDLE m_kick = nullptr; - HANDLE m_thread = nullptr; - DWORD m_threadId = 0; - uint64_t m_epoch = 0; - uint64_t m_instance = 0; + HANDLE m_mapping = nullptr; + ClipboardMapping * m_view = nullptr; + ClipboardRing * m_in = nullptr; + ClipboardRing * m_out = nullptr; + HANDLE m_stop = nullptr; + HANDLE m_kick = nullptr; + HANDLE m_writeReady = nullptr; + HANDLE m_thread = nullptr; + DWORD m_threadId = 0; + uint64_t m_epoch = 0; + uint64_t m_instance = 0; bool m_deferredDetach = false; IClipboardChannelHandler * m_handler = nullptr; @@ -97,7 +108,8 @@ private: void CleanupDeferredDetach(); void PublishState(bool available, uint64_t epoch); ClipboardChannelResult PublishRecord( - const KVMFRClipboardMessage& record, const uint8_t * data); + const KVMFRClipboardMessage& record, + std::vector&& data); void PublishReset(uint64_t epoch, uint32_t reason); static DWORD WINAPI ThreadProc(void * context); @@ -114,10 +126,17 @@ public: void Detach(); void Kick(uint64_t epoch); void Reset(uint64_t epoch, uint32_t reason); + bool WaitWritable(HANDLE stop, DWORD timeout); ClipboardChannelResult Send( const KVMFRClipboardMessage& record, const void * data = nullptr); + // Publish an ordered prefix of records, bounded by the physical ring + // window. BUSY may return a non-zero accepted count; the caller must resume + // at that prefix boundary. A single doorbell covers the entire prefix. + ClipboardChannelResult SendBatch(const ClipboardChannelWrite * records, + size_t count, size_t& accepted); + void SetHandler(IClipboardChannelHandler * handler); void ClearHandler(IClipboardChannelHandler * handler); diff --git a/idd/LGIdd/transport/CClipboardHub.cpp b/idd/LGIdd/transport/CClipboardHub.cpp index 951df152..902e9cf1 100644 --- a/idd/LGIdd/transport/CClipboardHub.cpp +++ b/idd/LGIdd/transport/CClipboardHub.cpp @@ -326,7 +326,7 @@ void CClipboardHub::ClipboardState(bool available, uint64_t epoch) } ClipboardChannelResult CClipboardHub::ClipboardRecord( - const KVMFRClipboardMessage& record, const uint8_t * data) + const KVMFRClipboardMessage& record, std::vector&& data) { if (!ValidHelperDirection(record)) return ClipboardChannelResult::FAILED; @@ -347,7 +347,8 @@ ClipboardChannelResult CClipboardHub::ClipboardRecord( KVMFRClipboardMessage stamped = record; stamped.generation = generation; const ClipboardChannelResult result = - source->SendClipboard(stamped, data); + source->SendClipboard(stamped, + data.empty() ? nullptr : data.data()); if (result != ClipboardChannelResult::FAILED) return result; diff --git a/idd/LGIdd/transport/CClipboardHub.h b/idd/LGIdd/transport/CClipboardHub.h index 35f74be8..a894d5af 100644 --- a/idd/LGIdd/transport/CClipboardHub.h +++ b/idd/LGIdd/transport/CClipboardHub.h @@ -55,7 +55,7 @@ private: void ClipboardState(bool available, uint64_t epoch) override; ClipboardChannelResult ClipboardRecord( const KVMFRClipboardMessage& record, - const uint8_t * data) override; + std::vector&& data) override; void ClipboardReset(uint64_t epoch, uint32_t reason) override; public: diff --git a/idd/LGIddHelper/CClipboardManager.cpp b/idd/LGIddHelper/CClipboardManager.cpp index f3d7b7be..abd2d8d6 100644 --- a/idd/LGIddHelper/CClipboardManager.cpp +++ b/idd/LGIddHelper/CClipboardManager.cpp @@ -1737,7 +1737,7 @@ void CClipboardManager::ProcessSend(Work&& work) if (result == ClipboardChannelResult::BUSY && GetTickCount64() < work.deadline && - WaitForSingleObject(m_stop, CHANNEL_RETRY_MS) != WAIT_OBJECT_0) + m_channel.WaitWritable(m_stop, CHANNEL_RETRY_MS)) { if (work.record.type == KVMFR_CLIPBOARD_MESSAGE_CANCEL) QueueCancel(work.record, work.record.token, work.deadline); @@ -1824,15 +1824,17 @@ void CClipboardManager::ProcessSendData(Work&& work) QueueCancel(work.record, ERROR_INVALID_DATA); return; } - const size_t length = static_cast((std::min)( - KVMFR_CLIPBOARD_REPRESENTATION_BYTES, - total - work.record.offset)); + + const uint64_t batchBytes = (std::min)( + static_cast(KVMFR_CLIPBOARD_REPRESENTATION_BYTES) * + KVMFR_CLIPBOARD_SLOT_COUNT, + total - work.record.offset); std::vector data; - if (length) + if (batchBytes) { try { - data.resize(length); + data.resize(static_cast(batchBytes)); } catch (const std::bad_alloc&) { @@ -1840,7 +1842,7 @@ void CClipboardManager::ProcessSendData(Work&& work) QueueCancel(work.record, ERROR_OUTOFMEMORY); return; } - if (!work.spool->Read(work.record.offset, data.data(), length)) + if (!work.spool->Read(work.record.offset, data.data(), data.size())) { const DWORD error = GetLastError(); ReleaseOutgoing(work.record.transfer); @@ -1849,49 +1851,82 @@ void CClipboardManager::ProcessSendData(Work&& work) } } - KVMFRClipboardMessage message = work.record; - message.type = KVMFR_CLIPBOARD_MESSAGE_DATA; - message.token = 0; - message.length = static_cast(length); - message.flags = 0; - if (!message.offset) - message.flags |= KVMFR_CLIPBOARD_FLAG_BEGIN; - if (message.offset + length == total) - message.flags |= KVMFR_CLIPBOARD_FLAG_END; - message.size = message.flags & KVMFR_CLIPBOARD_FLAG_END ? total : - (message.flags & KVMFR_CLIPBOARD_FLAG_BEGIN ? total : - KVMFR_CLIPBOARD_SIZE_UNKNOWN); + std::array batch; + size_t batchCount = 0; + uint64_t offset = work.record.offset; + uint32_t sequence = work.record.sequence; + do + { + ClipboardChannelWrite& write = batch[batchCount++]; + const size_t length = static_cast((std::min)( + KVMFR_CLIPBOARD_REPRESENTATION_BYTES, total - offset)); + write.record = work.record; + write.record.type = KVMFR_CLIPBOARD_MESSAGE_DATA; + write.record.token = 0; + write.record.offset = offset; + write.record.sequence = sequence; + write.record.length = static_cast(length); + write.record.flags = 0; + if (!offset) + write.record.flags |= KVMFR_CLIPBOARD_FLAG_BEGIN; + if (offset + length == total) + write.record.flags |= KVMFR_CLIPBOARD_FLAG_END; + write.record.size = + write.record.flags & KVMFR_CLIPBOARD_FLAG_END ? total : + (write.record.flags & KVMFR_CLIPBOARD_FLAG_BEGIN ? total : + KVMFR_CLIPBOARD_SIZE_UNKNOWN); + write.data = length ? data.data() + + static_cast(offset - work.record.offset) : nullptr; + offset += length; + ++sequence; + } + while (batchCount < batch.size() && offset < total); ClipboardChannelResult result = ClipboardChannelResult::FAILED; + size_t accepted = 0; bool stale = false; bool timedOut = false; { std::lock_guard lock(m_outgoingLock); stale = Atomic::Load(m_outgoingTransfer, std::memory_order_acquire) != - message.transfer || - message.clipboardGeneration != Atomic::Load( + work.record.transfer || + work.record.clipboardGeneration != Atomic::Load( m_liveLocalGeneration, std::memory_order_acquire); timedOut = GetTickCount64() >= work.deadline; if (!stale && !timedOut) - result = m_channel.Send(message, - data.empty() ? nullptr : data.data()); + result = m_channel.SendBatch(batch.data(), batchCount, accepted); } if (stale) { - ReleaseOutgoing(message.transfer); - QueueCancel(message, ERROR_OPERATION_ABORTED); + ReleaseOutgoing(work.record.transfer); + QueueCancel(work.record, ERROR_OPERATION_ABORTED); return; } if (timedOut) { - ReleaseOutgoing(message.transfer); - QueueCancel(message, ERROR_TIMEOUT); + ReleaseOutgoing(work.record.transfer); + QueueCancel(work.record, ERROR_TIMEOUT); return; } + + KVMFRClipboardMessage progress = work.record; + if (accepted) + { + progress = batch[accepted - 1U].record; + work.record.offset = progress.offset + progress.length; + work.record.sequence = progress.sequence + 1U; + work.deadline = GetTickCount64() + SEND_TIMEOUT_MS; + if (progress.flags & KVMFR_CLIPBOARD_FLAG_END) + { + ReleaseOutgoing(progress.transfer); + return; + } + } + if (result == ClipboardChannelResult::BUSY) { if (GetTickCount64() < work.deadline && - WaitForSingleObject(m_stop, CHANNEL_RETRY_MS) != WAIT_OBJECT_0) + m_channel.WaitWritable(m_stop, CHANNEL_RETRY_MS)) { const KVMFRClipboardMessage record = work.record; if (!QueueWork(std::move(work))) @@ -1909,19 +1944,11 @@ void CClipboardManager::ProcessSendData(Work&& work) } if (result != ClipboardChannelResult::ACCEPTED) { - ReleaseOutgoing(work.record.transfer); - QueueCancel(work.record, ERROR_DEVICE_NOT_CONNECTED); - return; - } - work.deadline = GetTickCount64() + SEND_TIMEOUT_MS; - if (message.flags & KVMFR_CLIPBOARD_FLAG_END) - { - ReleaseOutgoing(work.record.transfer); + ReleaseOutgoing(progress.transfer); + QueueCancel(progress, ERROR_DEVICE_NOT_CONNECTED); return; } - work.record.offset += length; - ++work.record.sequence; const KVMFRClipboardMessage record = work.record; if (!QueueWork(std::move(work))) { @@ -1985,52 +2012,75 @@ void CClipboardManager::ProcessSendFileData(Work&& work) QueueFileCancel(work.record, KVMFR_CLIPBOARD_FILE_ERROR_INVALID); return; } - const size_t length = static_cast((std::min)( - KVMFR_CLIPBOARD_DATA_BYTES, total - work.record.offset)); - KVMFRClipboardMessage message = work.record; - message.version = KVMFR_CLIPBOARD_VERSION; - message.type = KVMFR_CLIPBOARD_MESSAGE_FILE_DATA; - message.length = static_cast(length); - message.flags = 0; - if (!message.offset) - message.flags |= KVMFR_CLIPBOARD_FLAG_BEGIN; - if (message.offset + length == total) - message.flags |= KVMFR_CLIPBOARD_FLAG_END; - message.size = message.flags & KVMFR_CLIPBOARD_FLAG_END ? total : - (message.flags & KVMFR_CLIPBOARD_FLAG_BEGIN ? total : - KVMFR_CLIPBOARD_SIZE_UNKNOWN); - const uint8_t * data = length ? - work.fileData->data() + static_cast(message.offset) : nullptr; - const ClipboardChannelResult result = m_channel.Send(message, data); + std::array batch; + size_t batchCount = 0; + uint64_t offset = work.record.offset; + uint32_t sequence = work.record.sequence; + do + { + ClipboardChannelWrite& write = batch[batchCount++]; + const size_t length = static_cast((std::min)( + KVMFR_CLIPBOARD_DATA_BYTES, total - offset)); + write.record = work.record; + write.record.version = KVMFR_CLIPBOARD_VERSION; + write.record.type = KVMFR_CLIPBOARD_MESSAGE_FILE_DATA; + write.record.offset = offset; + write.record.sequence = sequence; + write.record.length = static_cast(length); + write.record.flags = 0; + if (!offset) + write.record.flags |= KVMFR_CLIPBOARD_FLAG_BEGIN; + if (offset + length == total) + write.record.flags |= KVMFR_CLIPBOARD_FLAG_END; + write.record.size = + write.record.flags & KVMFR_CLIPBOARD_FLAG_END ? total : + (write.record.flags & KVMFR_CLIPBOARD_FLAG_BEGIN ? total : + KVMFR_CLIPBOARD_SIZE_UNKNOWN); + write.data = length ? work.fileData->data() + + static_cast(offset) : nullptr; + offset += length; + ++sequence; + } + while (batchCount < batch.size() && offset < total); + + size_t accepted = 0; + const ClipboardChannelResult result = + m_channel.SendBatch(batch.data(), batchCount, accepted); + KVMFRClipboardMessage progress = work.record; + if (accepted) + { + progress = batch[accepted - 1U].record; + work.record.offset = progress.offset + progress.length; + work.record.sequence = progress.sequence + 1U; + work.deadline = GetTickCount64() + SEND_TIMEOUT_MS; + if (progress.flags & KVMFR_CLIPBOARD_FLAG_END) + { + std::lock_guard lock(m_fileLock); + m_outgoingFileRequests.erase(progress.transfer); + return; + } + } + if (result == ClipboardChannelResult::BUSY && GetTickCount64() < work.deadline && - WaitForSingleObject(m_stop, CHANNEL_RETRY_MS) != WAIT_OBJECT_0) + m_channel.WaitWritable(m_stop, CHANNEL_RETRY_MS)) { if (!QueueWork(std::move(work))) - QueueFileCancel(message, KVMFR_CLIPBOARD_FILE_ERROR_NO_MEMORY); + QueueFileCancel(progress, KVMFR_CLIPBOARD_FILE_ERROR_NO_MEMORY); return; } if (result != ClipboardChannelResult::ACCEPTED) { - QueueFileCancel(message, + QueueFileCancel(progress, result == ClipboardChannelResult::BUSY ? KVMFR_CLIPBOARD_FILE_ERROR_IO : KVMFR_CLIPBOARD_FILE_ERROR_DISCONNECTED); return; } - if (message.flags & KVMFR_CLIPBOARD_FLAG_END) - { - std::lock_guard lock(m_fileLock); - m_outgoingFileRequests.erase(message.transfer); - return; - } - work.record.offset += length; - ++work.record.sequence; - work.deadline = GetTickCount64() + SEND_TIMEOUT_MS; if (!QueueWork(std::move(work))) - QueueFileCancel(message, KVMFR_CLIPBOARD_FILE_ERROR_NO_MEMORY); + QueueFileCancel(progress, KVMFR_CLIPBOARD_FILE_ERROR_NO_MEMORY); } void CClipboardManager::ProcessFileRecord( @@ -2423,15 +2473,30 @@ void CClipboardManager::ProcessFileData( if (valid && record.length) { - try + if (request->operation == KVMFR_CLIPBOARD_FILE_OP_READ && + request->output) { - request->data.insert(request->data.end(), data, - data + record.length); + if (request->nextOffset > request->outputCapacity || + record.length > + request->outputCapacity - request->nextOffset) + valid = false; + else + memcpy(request->output + + static_cast(request->nextOffset), data, + record.length); } - catch (const std::bad_alloc&) + else { - valid = false; - request->error = KVMFR_CLIPBOARD_FILE_ERROR_NO_MEMORY; + try + { + request->data.insert(request->data.end(), data, + data + record.length); + } + catch (const std::bad_alloc&) + { + valid = false; + request->error = KVMFR_CLIPBOARD_FILE_ERROR_NO_MEMORY; + } } } if (valid) @@ -2809,6 +2874,8 @@ HRESULT CClipboardManager::ReadRemoteFile(uint64_t dataset, read = 0; if (Atomic::Load(m_shutdown)) return STG_E_READFAULT; + if (!output && length) + return STG_E_INVALIDPOINTER; uint8_t * destination = static_cast(output); while (length) { @@ -2831,6 +2898,8 @@ HRESULT CClipboardManager::ReadRemoteFile(uint64_t dataset, request->node = node; request->operation = KVMFR_CLIPBOARD_FILE_OP_READ; request->requestedBytes = wanted; + request->output = destination; + request->outputCapacity = wanted; KVMFRClipboardFileError insertError = KVMFR_CLIPBOARD_FILE_ERROR_NONE; { @@ -2906,11 +2975,9 @@ HRESULT CClipboardManager::ReadRemoteFile(uint64_t dataset, if (request->error != KVMFR_CLIPBOARD_FILE_ERROR_NONE) return request->error == KVMFR_CLIPBOARD_FILE_ERROR_ACCESS ? STG_E_ACCESSDENIED : STG_E_READFAULT; - if (request->data.size() > wanted) + if (request->nextOffset > wanted) return STG_E_READFAULT; - if (!request->data.empty()) - memcpy(destination, request->data.data(), request->data.size()); - const ULONG actual = static_cast(request->data.size()); + const ULONG actual = static_cast(request->nextOffset); destination += actual; read += actual; offset += actual; @@ -2996,8 +3063,11 @@ void CClipboardManager::ClipboardState(bool available, uint64_t epoch) } ClipboardChannelResult CClipboardManager::ClipboardRecord( - const KVMFRClipboardMessage& record, const uint8_t * data) + const KVMFRClipboardMessage& record, std::vector&& data) { + if (data.size() != record.length) + return ClipboardChannelResult::FAILED; + switch (record.type) { case KVMFR_CLIPBOARD_MESSAGE_OFFER: @@ -3028,8 +3098,7 @@ ClipboardChannelResult CClipboardManager::ClipboardRecord( case KVMFR_CLIPBOARD_MESSAGE_DATA: if (!record.clipboardGeneration || !record.transfer || !kvmfrClipboardTransferFromHelper(record.transfer) || - !kvmfrClipboardRepresentationFormatValid(record.format) || - (record.length && !data)) + !kvmfrClipboardRepresentationFormatValid(record.format)) return ClipboardChannelResult::FAILED; break; @@ -3056,8 +3125,7 @@ ClipboardChannelResult CClipboardManager::ClipboardRecord( (record.type == KVMFR_CLIPBOARD_MESSAGE_FILE_REQUEST && !kvmfrClipboardTransferFromClient(record.transfer)) || (record.type == KVMFR_CLIPBOARD_MESSAGE_FILE_DATA && - !kvmfrClipboardTransferFromHelper(record.transfer)) || - (record.length && !data)) + !kvmfrClipboardTransferFromHelper(record.transfer))) return ClipboardChannelResult::FAILED; break; default: @@ -3068,16 +3136,7 @@ ClipboardChannelResult CClipboardManager::ClipboardRecord( work.type = WorkType::RECORD; work.record = record; if (record.length) - { - try - { - work.data.assign(data, data + record.length); - } - catch (const std::bad_alloc&) - { - return ClipboardChannelResult::BUSY; - } - } + work.data = std::move(data); if (QueueWork(std::move(work))) return ClipboardChannelResult::ACCEPTED; return Atomic::Load(m_shutdown) ? ClipboardChannelResult::FAILED : diff --git a/idd/LGIddHelper/CClipboardManager.h b/idd/LGIddHelper/CClipboardManager.h index 21bae4a8..7b37ec55 100644 --- a/idd/LGIddHelper/CClipboardManager.h +++ b/idd/LGIddHelper/CClipboardManager.h @@ -138,6 +138,10 @@ private: bool manifest = false; KVMFRClipboardFileError error = KVMFR_CLIPBOARD_FILE_ERROR_NONE; HANDLE event = nullptr; + // Borrowed by a synchronous READ until this request is removed while + // holding m_fileLock. Manifest LIST responses retain owned vector data. + uint8_t * output = nullptr; + uint32_t outputCapacity = 0; std::vector data; ~IncomingFileRequest(); @@ -322,8 +326,9 @@ private: CClipboardSpool& spool); void ClipboardState(bool available, uint64_t epoch) override; - ClipboardChannelResult ClipboardRecord(const KVMFRClipboardMessage& record, - const uint8_t * data) override; + ClipboardChannelResult ClipboardRecord( + const KVMFRClipboardMessage& record, + std::vector&& data) override; void ClipboardReset(uint64_t epoch, uint32_t reason) override; public: