From 59b7d129a18681fc37ca59b4ba2809b93b890d5f Mon Sep 17 00:00:00 2001
From: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
Date: Tue, 22 Aug 2023 22:17:37 +0200
Subject: [PATCH] Improve error message in common `re_renderer` crash (#3070)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Here is a reproduce:

```diff
--- a/examples/rust/clock/src/main.rs
+++ b/examples/rust/clock/src/main.rs
@@ -77,7 +77,7 @@ fn run(rec_stream: &RecordingStream, args: &Args) -> anyhow::Result<()> {
             .with_component(&[color])?
             .send(rec_stream)?;
         MsgSender::new(format!("world/{name}_hand"))
-            .with_component(&[Vector3D::from(pos)])?
+            .with_component(&[Vector3D::from(pos); 65537])?
             .with_component(&[color])?
             .with_component(&[Radius(width * 0.5)])?
             .send(rec_stream)?;
```

We are hitting `LineDrawData::MAX_NUM_STRIPS`.

It's really bad if it is the case that hitting an upper limit crashes
the viewer 😭


----

In 0.8.1 we have 21 users crashing in
`re_renderer-0.8.0/src/allocator/cpu_write_gpu_read_belt.rs:87`
Callstack:

```
   8: <re_renderer::line_strip_builder::LineStripBuilder as core::ops::drop::Drop>::drop
   9: re_space_view_spatial::parts::arrows3d::Arrows3DPart::process_entity_view
  10: re_space_view_spatial::parts::entity_iterator::process_entity_views
  11: <re_space_view_spatial::parts::arrows3d::Arrows3DPart as re_viewer_context::space_view::view_part_system::ViewPartSystem>::execute
  12: re_viewer_context::space_view::space_view_class::<impl re_viewer_context::space_view::dyn_space_view_class::DynSpaceViewClass for T>::ui
  13: core::ops::function::FnOnce::call_once{{vtable.shim}}
```

`Arrows3DPart` is using `LineBatchBuilder`.
`LineBatchBuilder::add_segment` returns a `LineStripBuilder`
`LineStripBuilder::drop` calls `CpuWriteGpuReadBuffer::extend` which
calls
`CpuWriteGpuReadBuffer::push` which crashes because the
`CpuWriteGpuReadBuffer` is full

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/{{
pr.number }}) (if applicable)

- [PR Build Summary](https://build.rerun.io/pr/{{ pr.number }})
- [Docs preview](https://rerun.io/preview/{{
"pr:%s"|format(pr.branch)|encode_uri_component }}/docs)
- [Examples preview](https://rerun.io/preview/{{
"pr:%s"|format(pr.branch)|encode_uri_component }}/examples)
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
---
 .../src/allocator/cpu_write_gpu_read_belt.rs       | 14 +++++++++++---
 scripts/fetch_crashes.py                           |  1 +
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/crates/re_renderer/src/allocator/cpu_write_gpu_read_belt.rs b/crates/re_renderer/src/allocator/cpu_write_gpu_read_belt.rs
index a1bfa23901c70..4a1793836abe4 100644
--- a/crates/re_renderer/src/allocator/cpu_write_gpu_read_belt.rs
+++ b/crates/re_renderer/src/allocator/cpu_write_gpu_read_belt.rs
@@ -49,7 +49,7 @@ where
     ///
     /// Do *not* make this public as we need to guarantee that the memory is *never* read from!
     #[inline(always)]
-    fn as_slice(&mut self) -> &mut [u8] {
+    fn as_mut_byte_slice(&mut self) -> &mut [u8] {
         // TODO(andreas): Is this access slow given that it internally goes through a trait interface? Should we keep the pointer around?
         &mut self.write_view[self.unwritten_element_range.start * std::mem::size_of::<T>()
             ..self.unwritten_element_range.end * std::mem::size_of::<T>()]
@@ -61,7 +61,7 @@ where
     #[inline]
     pub fn extend_from_slice(&mut self, elements: &[T]) {
         let bytes = bytemuck::cast_slice(elements);
-        self.as_slice()[..bytes.len()].copy_from_slice(bytes);
+        self.as_mut_byte_slice()[..bytes.len()].copy_from_slice(bytes);
         self.unwritten_element_range.start += elements.len();
     }
 
@@ -84,7 +84,15 @@ where
     /// Panics if the data no longer fits into the buffer.
     #[inline]
     pub fn push(&mut self, element: T) {
-        self.as_slice()[..std::mem::size_of::<T>()].copy_from_slice(bytemuck::bytes_of(&element));
+        assert!(
+            self.unwritten_element_range.start < self.unwritten_element_range.end,
+            "CpuWriteGpuReadBuffer<{}> is full ({} elements written)",
+            std::any::type_name::<T>(),
+            self.unwritten_element_range.start,
+        );
+
+        self.as_mut_byte_slice()[..std::mem::size_of::<T>()]
+            .copy_from_slice(bytemuck::bytes_of(&element));
         self.unwritten_element_range.start += 1;
     }
 
diff --git a/scripts/fetch_crashes.py b/scripts/fetch_crashes.py
index 1c9d417f03200..577d6beac4ba8 100755
--- a/scripts/fetch_crashes.py
+++ b/scripts/fetch_crashes.py
@@ -163,4 +163,5 @@ def count_uniques(backtrace):
         "```\n"
         f'   {backtrace.decode("utf-8")}\n'
         "```\n"
+        "-------------------------------------------------------------------------------\n"
     )