Comment

Comment by tekacs

To pre-empt the folks who'll come in here and laugh about how Rust should be preventing memory corruption... I'll just directly quote from the mailing list:

  Rust Binder contains the following unsafe operation:
  
   // SAFETY: A `NodeDeath` is never inserted into the death list
   // of any node other than its owner, so it is either in this
   // death list or in no death list.
   unsafe { node_inner.death_list.remove(self) };
  
  This operation is unsafe because when touching the prev/next pointers of
  a list element, we have to ensure that no other thread is also touching
  them in parallel. If the node is present in the list that `remove` is
  called on, then that is fine because we have exclusive access to that
  list. If the node is not in any list, then it's also ok. But if it's
  present in a different list that may be accessed in parallel, then that
  may be a data race on the prev/next pointers.
  
  And unfortunately that is exactly what is happening here. In
  Node::release, we:
  
   1. Take the lock.
   2. Move all items to a local list on the stack.
   3. Drop the lock.
   4. Iterate the local list on the stack.
  
  Combined with threads using the unsafe remove method on the original
  list, this leads to memory corruption of the prev/next pointers. This
  leads to crashes like this one:
parent tekacs 10 months ago

Discussion

Live from the official Hacker News API

A modern reader for Hacker News · SvelteKit + Skeleton