logoalt Hacker News

anon-3988yesterday at 7:16 PM2 repliesview on HN

The interesting part to me is that this bug does not necessarily happen in an unsafe block. The fix happens in an unsafe block, I think the API should change to avoid this. Perhaps by forcing users to pass a lambda to do stuff instead of having to manually lock and drop?


Replies

aw1621107yesterday at 7:45 PM

The `unsafe` block was present because `List::remove` is marked `unsafe` [0]:

    /// Removes the provided item from this list and returns it.
    ///
    /// This returns `None` if the item is not in the list. (Note that by the safety requirements,
    /// this means that the item is not in any list.)
    ///
    /// # Safety
    ///
    /// `item` must not be in a different linked list (with the same id).
    pub unsafe fn remove(&mut self, item: &T) -> Option<ListArc<T, ID>> {
I think it'd be tricky at best to make this particular API safe since doing so requires reasoning across arbitrary other List instances. At the very least I don't think locks would help here, since temporary exclusive access to a list won't stop you from adding the same element to multiple lists.

[0]: https://github.com/torvalds/linux/blob/3e0ae02ba831da2b70790...

show 1 reply
mlindneryesterday at 8:43 PM

Yeah this is a bad fix. It should be impossible to cause incorrect things to happen from safe code, especially from safe code calling safe code.

show 1 reply