Skip to content

Commit 5fc9fef

Browse files
authored
refactor(server): remove needless sync primitives (#911)
Instead use a `Option` as a slot from which the value can be moved out temporarily when necessary.
1 parent 85fc1c7 commit 5fc9fef

1 file changed

Lines changed: 26 additions & 18 deletions

File tree

  • crates/ironrdp-server/src/encoder

crates/ironrdp-server/src/encoder/mod.rs

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
use core::fmt;
22
use core::num::NonZeroU16;
3-
use std::sync::{Arc, Mutex};
43

5-
use anyhow::{anyhow, Context as _, Result};
4+
use anyhow::{Context as _, Result};
65
use ironrdp_acceptor::DesktopSize;
76
use ironrdp_graphics::diff::{find_different_rects_sub, Rect};
87
use ironrdp_pdu::encode_vec;
@@ -80,7 +79,7 @@ impl Default for UpdateEncoderCodecs {
8079
pub(crate) struct UpdateEncoder {
8180
desktop_size: DesktopSize,
8281
framebuffer: Option<Framebuffer>,
83-
bitmap_updater: BitmapUpdater,
82+
bitmap_updater: Option<BitmapUpdater>,
8483
}
8584

8685
impl fmt::Debug for UpdateEncoder {
@@ -118,7 +117,7 @@ impl UpdateEncoder {
118117
Self {
119118
desktop_size,
120119
framebuffer: None,
121-
bitmap_updater,
120+
bitmap_updater: Some(bitmap_updater),
122121
}
123122
}
124123

@@ -132,7 +131,10 @@ impl UpdateEncoder {
132131

133132
pub(crate) fn set_desktop_size(&mut self, size: DesktopSize) {
134133
self.desktop_size = size;
135-
self.bitmap_updater.set_desktop_size(size);
134+
self.bitmap_updater
135+
.as_mut()
136+
.expect("bitmap updater always Some")
137+
.set_desktop_size(size);
136138
}
137139

138140
fn rgba_pointer(ptr: RGBAPointer) -> Result<UpdateFragmenter> {
@@ -235,12 +237,20 @@ impl UpdateEncoder {
235237
}
236238

237239
async fn bitmap(&mut self, bitmap: BitmapUpdate) -> Result<UpdateFragmenter> {
238-
// Clone to satisfy spawn_blocking 'static requirement
239-
// this should be cheap, even if using bitmap, since vec![] will be empty
240-
let mut updater = self.bitmap_updater.clone();
241-
tokio::task::spawn_blocking(move || time_warn!("Encoding bitmap", 10, updater.handle(&bitmap)))
242-
.await
243-
.unwrap()
240+
// Move the bitmap updater to satisfy spawn_blocking 'static requirement.
241+
// It is restored after the blocking operation completes.
242+
let mut updater = self.bitmap_updater.take().expect("bitmap updater always Some");
243+
244+
let (result, updater) = tokio::task::spawn_blocking(move || {
245+
let result = time_warn!("Encoding bitmap", 10, updater.handle(&bitmap));
246+
(result, updater)
247+
})
248+
.await
249+
.unwrap();
250+
251+
self.bitmap_updater = Some(updater);
252+
253+
result
244254
}
245255
}
246256

@@ -314,7 +324,7 @@ impl EncoderIter<'_> {
314324
}
315325
}
316326

317-
#[derive(Debug, Clone)]
327+
#[derive(Debug)]
318328
enum BitmapUpdater {
319329
None(NoneHandler),
320330
Bitmap(BitmapHandler),
@@ -470,10 +480,9 @@ impl BitmapUpdateHandler for QoiHandler {
470480
}
471481

472482
#[cfg(feature = "qoiz")]
473-
#[derive(Clone)]
474483
struct QoizHandler {
475484
codec_id: u8,
476-
zctxt: Arc<Mutex<zstd_safe::CCtx<'static>>>,
485+
zctxt: zstd_safe::CCtx<'static>,
477486
}
478487

479488
#[cfg(feature = "qoiz")]
@@ -492,7 +501,6 @@ impl QoizHandler {
492501
zctxt
493502
.set_parameter(zstd_safe::CParameter::EnableLongDistanceMatching(true))
494503
.unwrap();
495-
let zctxt = Arc::new(Mutex::new(zctxt));
496504

497505
Self { codec_id, zctxt }
498506
}
@@ -507,16 +515,16 @@ impl BitmapUpdateHandler for QoizHandler {
507515
let mut outb;
508516
let mut pos = 0;
509517

510-
let mut zctxt = self.zctxt.lock().unwrap();
511518
loop {
512519
outb = zstd_safe::OutBuffer::around_pos(data.as_mut_slice(), pos);
513-
let res = zctxt
520+
let res = self
521+
.zctxt
514522
.compress_stream2(
515523
&mut outb,
516524
&mut inb,
517525
zstd_safe::zstd_sys::ZSTD_EndDirective::ZSTD_e_flush,
518526
)
519-
.map_err(|code| anyhow!("failed to zstd compress: {}", zstd_safe::get_error_name(code)))?;
527+
.map_err(|code| anyhow::anyhow!("failed to zstd compress: {}", zstd_safe::get_error_name(code)))?;
520528
if res == 0 {
521529
break;
522530
}

0 commit comments

Comments
 (0)