Skip to content

Commit

Permalink
Improve error message in common re_renderer crash (#3070)
Browse files Browse the repository at this point in the history
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/)
  • Loading branch information
emilk authored and jleibs committed Aug 31, 2023
1 parent 31a2eb0 commit 59b7d12
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
14 changes: 11 additions & 3 deletions crates/re_renderer/src/allocator/cpu_write_gpu_read_belt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>()]
Expand All @@ -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();
}

Expand All @@ -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;
}

Expand Down
1 change: 1 addition & 0 deletions scripts/fetch_crashes.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,5 @@ def count_uniques(backtrace):
"```\n"
f' {backtrace.decode("utf-8")}\n'
"```\n"
"-------------------------------------------------------------------------------\n"
)

0 comments on commit 59b7d12

Please sign in to comment.