Re-add Sync (#242)

* Improved realloc
* Restore Sync

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lol_html"
-version = "2.0.0"
+version = "2.1.0"
 authors = ["Ivan Nikulin <inikulin@cloudflare.com, ifaaan@gmail.com>"]
 license = "BSD-3-Clause"
 description = "Streaming HTML rewriter/parser with CSS selector-based API"

src/memory/arena.rs

@@ -19,21 +19,21 @@ impl Arena {
     }
 
     pub fn append(&mut self, slice: &[u8]) -> Result<(), MemoryLimitExceededError> {
-        let new_len = self.data.len() + slice.len();
-        let capacity = self.data.capacity();
+        // this specific form of capacity check optimizes out redundant resizing in extend_from_slice
+        if self.data.capacity() - self.data.len() < slice.len() {
+            let additional = slice.len() + self.data.len() - self.data.capacity();
 
-        if new_len > capacity {
-            let additional = new_len - capacity;
-
-            // NOTE: approximate usage, as `Vec::reserve_exact` doesn't
+            // NOTE: approximate usage, as `Vec::(try_)reserve_exact` doesn't
             // give guarantees about exact capacity value :).
             self.limiter.increase_usage(additional)?;
 
             // NOTE: with wisely chosen preallocated size this branch should be
             // executed quite rarely. We can't afford to use double capacity
             // strategy used by default (see: https://github.com/rust-lang/rust/blob/bdfd698f37184da42254a03ed466ab1f90e6fb6c/src/liballoc/raw_vec.rs#L424)
             // as we'll run out of the space allowance quite quickly.
-            self.data.reserve_exact(slice.len());
+            self.data
+                .try_reserve_exact(slice.len())
+                .map_err(|_| MemoryLimitExceededError)?;
         }
 
         self.data.extend_from_slice(slice);

src/rewritable_units/element.rs

@@ -254,7 +254,7 @@ impl<'r, 't, H: HandlerTypes> Element<'r, 't, H> {
             .mutations
             .mutate()
             .content_before
-            .push_back(StringChunk::Stream(string_writer));
+            .push_back(StringChunk::stream(string_writer));
     }
 
     /// Inserts `content` after the element.
@@ -306,7 +306,7 @@ impl<'r, 't, H: HandlerTypes> Element<'r, 't, H> {
     ///
     /// Use the [`streaming!`] macro to make a `StreamingHandler` from a closure.
     pub fn streaming_after(&mut self, string_writer: Box<dyn StreamingHandler + Send>) {
-        self.after_chunk(StringChunk::Stream(string_writer));
+        self.after_chunk(StringChunk::stream(string_writer));
     }
 
     /// Prepends `content` to the element's inner content, i.e. inserts content right after
@@ -370,7 +370,7 @@ impl<'r, 't, H: HandlerTypes> Element<'r, 't, H> {
     ///
     /// Use the [`streaming!`] macro to make a `StreamingHandler` from a closure.
     pub fn streaming_prepend(&mut self, string_writer: Box<dyn StreamingHandler + Send>) {
-        self.prepend_chunk(StringChunk::Stream(string_writer));
+        self.prepend_chunk(StringChunk::stream(string_writer));
     }
 
     /// Appends `content` to the element's inner content, i.e. inserts content right before
@@ -429,7 +429,7 @@ impl<'r, 't, H: HandlerTypes> Element<'r, 't, H> {
     ///
     /// Use the [`streaming!`] macro to make a `StreamingHandler` from a closure.
     pub fn streaming_append(&mut self, string_writer: Box<dyn StreamingHandler + Send>) {
-        self.append_chunk(StringChunk::Stream(string_writer));
+        self.append_chunk(StringChunk::stream(string_writer));
     }
 
     /// Replaces inner content of the element with `content`.
@@ -492,7 +492,7 @@ impl<'r, 't, H: HandlerTypes> Element<'r, 't, H> {
     ///
     /// Use the [`streaming!`] macro to make a `StreamingHandler` from a closure.
     pub fn streaming_set_inner_content(&mut self, string_writer: Box<dyn StreamingHandler + Send>) {
-        self.set_inner_content_chunk(StringChunk::Stream(string_writer));
+        self.set_inner_content_chunk(StringChunk::stream(string_writer));
     }
 
     /// Replaces the element and its inner content with `content`.
@@ -543,7 +543,7 @@ impl<'r, 't, H: HandlerTypes> Element<'r, 't, H> {
     ///
     /// Use the [`streaming!`] macro to make a `StreamingHandler` from a closure.
     pub fn streaming_replace(&mut self, string_writer: Box<dyn StreamingHandler + Send>) {
-        self.replace_chunk(StringChunk::Stream(string_writer));
+        self.replace_chunk(StringChunk::stream(string_writer));
     }
 
     /// Removes the element and its inner content.

src/rewritable_units/mutations.rs

@@ -1,6 +1,7 @@
 use super::StreamingHandlerSink;
 use std::error::Error as StdError;
 use std::panic::{RefUnwindSafe, UnwindSafe};
+use std::sync::Mutex;
 
 type BoxResult = Result<(), Box<dyn StdError + Send + Sync>>;
 
@@ -84,17 +85,25 @@ impl Mutations {
     }
 }
 
+/// Part of [`DynamicString`]
 pub(crate) enum StringChunk {
     Buffer(Box<str>, ContentType),
-    Stream(Box<dyn StreamingHandler + Send>),
+    // The mutex is never actually locked, but makes the struct `Sync` without unsafe.
+    Stream(Mutex<Box<dyn StreamingHandler + Send>>),
 }
 
 impl StringChunk {
     pub(crate) fn from_str(content: impl Into<Box<str>>, content_type: ContentType) -> Self {
         Self::Buffer(content.into(), content_type)
     }
+
+    #[inline]
+    pub(crate) fn stream(handler: Box<dyn StreamingHandler + Send>) -> Self {
+        Self::Stream(Mutex::new(handler))
+    }
 }
 
+/// String built from fragments or dynamic callbacks
 #[derive(Default)]
 pub(crate) struct DynamicString {
     chunks: Vec<StringChunk>,
@@ -128,7 +137,11 @@ impl DynamicString {
                     sink.write_str(&content, content_type);
                 }
                 StringChunk::Stream(handler) => {
-                    handler.write_all(sink)?;
+                    // The mutex will never be locked or poisoned. This is the cheapest way to unwrap it.
+                    let (Ok(h) | Err(h)) = handler
+                        .into_inner()
+                        .map_err(std::sync::PoisonError::into_inner);
+                    h.write_all(sink)?;
                 }
             };
         }
@@ -145,8 +158,6 @@ pub trait StreamingHandler {
     ///
     /// See [`StreamingHandlerSink`].
     fn write_all(self: Box<Self>, sink: &mut StreamingHandlerSink<'_>) -> BoxResult;
-
-    // Safety: due to lack of Sync, this trait must not have `&self` methods
 }
 
 impl RefUnwindSafe for StringChunk {}

src/rewritable_units/tokens/comment.rs

@@ -124,7 +124,7 @@ impl<'i> Comment<'i> {
         self.mutations
             .mutate()
             .content_before
-            .push_back(StringChunk::Stream(string_writer));
+            .push_back(StringChunk::stream(string_writer));
     }
 
     /// Inserts `content` after the comment.
@@ -172,7 +172,7 @@ impl<'i> Comment<'i> {
         self.mutations
             .mutate()
             .content_after
-            .push_front(StringChunk::Stream(string_writer));
+            .push_front(StringChunk::stream(string_writer));
     }
 
     /// Replaces the comment with the `content`.
@@ -218,7 +218,7 @@ impl<'i> Comment<'i> {
     pub fn streaming_replace(&mut self, string_writer: Box<dyn StreamingHandler + Send>) {
         self.mutations
             .mutate()
-            .replace(StringChunk::Stream(string_writer));
+            .replace(StringChunk::stream(string_writer));
     }
 
     /// Removes the comment.

src/rewritable_units/tokens/end_tag.rs

@@ -107,7 +107,7 @@ impl<'i> EndTag<'i> {
         self.mutations
             .mutate()
             .content_before
-            .push_back(StringChunk::Stream(string_writer));
+            .push_back(StringChunk::stream(string_writer));
     }
 
     /// Inserts content from a [`StreamingHandler`] after the end tag.
@@ -120,7 +120,7 @@ impl<'i> EndTag<'i> {
         self.mutations
             .mutate()
             .content_after
-            .push_front(StringChunk::Stream(string_writer));
+            .push_front(StringChunk::stream(string_writer));
     }
 
     /// Replaces the end tag with content from a [`StreamingHandler`].
@@ -132,7 +132,7 @@ impl<'i> EndTag<'i> {
     pub fn streaming_replace(&mut self, string_writer: Box<dyn StreamingHandler + Send>) {
         self.mutations
             .mutate()
-            .replace(StringChunk::Stream(string_writer));
+            .replace(StringChunk::stream(string_writer));
     }
 
     /// Removes the end tag.

src/rewritable_units/tokens/start_tag.rs

@@ -149,7 +149,7 @@ impl<'i> StartTag<'i> {
         self.mutations
             .mutate()
             .content_before
-            .push_back(StringChunk::Stream(string_writer));
+            .push_back(StringChunk::stream(string_writer));
     }
 
     /// Inserts content from a [`StreamingHandler`] after the start tag.
@@ -161,7 +161,7 @@ impl<'i> StartTag<'i> {
         self.mutations
             .mutate()
             .content_after
-            .push_front(StringChunk::Stream(string_writer));
+            .push_front(StringChunk::stream(string_writer));
     }
 
     /// Replaces the start tag with the content from a [`StreamingHandler`].
@@ -172,7 +172,7 @@ impl<'i> StartTag<'i> {
     pub fn streaming_replace(&mut self, string_writer: Box<dyn StreamingHandler + Send>) {
         self.mutations
             .mutate()
-            .replace(StringChunk::Stream(string_writer));
+            .replace(StringChunk::stream(string_writer));
     }
 
     /// Removes the start tag.

src/rewritable_units/tokens/text_chunk.rs

@@ -275,7 +275,7 @@ impl<'i> TextChunk<'i> {
         self.mutations
             .mutate()
             .content_before
-            .push_back(StringChunk::Stream(string_writer));
+            .push_back(StringChunk::stream(string_writer));
     }
 
     /// Inserts content from a [`StreamingHandler`] after the text chunk.
@@ -287,7 +287,7 @@ impl<'i> TextChunk<'i> {
         self.mutations
             .mutate()
             .content_after
-            .push_front(StringChunk::Stream(string_writer));
+            .push_front(StringChunk::stream(string_writer));
     }
 
     /// Replaces the text chunk with the content from a [`StreamingHandler`].
@@ -298,7 +298,7 @@ impl<'i> TextChunk<'i> {
     pub fn streaming_replace(&mut self, string_writer: Box<dyn StreamingHandler + Send>) {
         self.mutations
             .mutate()
-            .replace(StringChunk::Stream(string_writer));
+            .replace(StringChunk::stream(string_writer));
     }
 
     /// Removes the text chunk.