Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack overflow at runtime when using Rc<RefCell<T>> in cycle detection algorithm #132263

Closed
Ayanekoji opened this issue Oct 28, 2024 · 3 comments
Closed
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues.

Comments

@Ayanekoji
Copy link

Ayanekoji commented Oct 28, 2024

I tried this code:

use std::{cell::RefCell, rc::Rc};

#[derive(Debug)]
struct CycleNode {
    pos: i32,
    next: Option<Rc<RefCell<CycleNode>>>,
}

impl CycleNode {
    fn new(pos: i32) -> Rc<RefCell<Self>> {
        Rc::new(RefCell::new(CycleNode { pos, next: None }))
    }
}

pub fn detect_cycle(head: Option<Rc<RefCell<CycleNode>>>) -> Option<Rc<RefCell<CycleNode>>> {
        if head.is_none() {
            return None;
        }

        let mut fast = head.clone();
        let mut slow = head.clone();

        while let (Some(fast_node), Some(slow_node)) = (fast, slow) {
            if fast_node.borrow().next.is_none()
                || fast_node
                    .borrow()
                    .next
                    .as_ref()
                    .unwrap()
                    .borrow()
                    .next
                    .is_none()
            {
                return None;
            }

            fast = fast_node
                .borrow()
                .next
                .as_ref()
                .unwrap()
                .borrow()
                .next
                .clone();
            slow = slow_node.borrow().next.clone();

            if let (Some(f), Some(s)) = (&fast, &slow) {
                if Rc::ptr_eq(f, s) {
                    let mut entry = head.clone();

                    while let (Some(entry_node), Some(fast_node)) = (&entry.clone(), &fast.clone())
                    {
                        if Rc::ptr_eq(entry_node, fast_node) {
                            return entry;
                        }

                        entry = entry_node.borrow().next.clone();
                        fast = fast_node.borrow().next.clone();
                    }
                }
            }
        }

        None
    }
     
fn main() {
   let nodes: Vec<Rc<RefCell<CycleNode>>> = (0..5)
        .map(|pos| CycleNode::new(rand::thread_rng().gen_range(1..10), pos))
        .collect();
    for i in 0..nodes.len() - 1 {
        nodes[i].borrow_mut().next = Some(nodes[i + 1].clone());
    }
    nodes[4].borrow_mut().next = Some(nodes[2].clone());
    let head = Some(nodes[0].clone());
    let result = Solution::detect_cycle(head);
    assert_eq!(result,Some(nodes[2].clone()));
print!("success");
}

I expected to see this happen: success

Instead, this happened:

wind@Unreal:~/Learning_Projects/Rust/leetcode$ cargo build
   Compiling leetcode v0.1.0 (/home/wind/Learning_Projects/Rust/leetcode)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.59s
wind@Unreal:~/Learning_Projects/Rust/leetcode$ cargo run
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.03s
     Running `/home/wind/Learning_Projects/Rust/target/debug/leetcode`

thread 'main' has overflowed its stack
fatal runtime error: stack overflow

Meta

rustc --version --verbose:

<version> wind@Unreal:~/Learning_Projects/Rust/leetcode$ rustc --version
rustc 1.82.0 (f6e511eec 2024-10-15)
Backtrace

<backtrace>

@Ayanekoji Ayanekoji added the C-bug Category: This is a bug. label Oct 28, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 28, 2024
@ShE3py
Copy link
Contributor

ShE3py commented Oct 28, 2024

Removing the assert_eq!(result, Some(nodes[2].clone())); solves the stack overflow; I guess the impl PartialEq for CycleNode loops infinitely ?

#93555 0x00005555555607c6 in RefCell<CycleNode>::eq (self=0x5555555f8c70, other=0x5555555f8c70)
// 2nd cycle ↑
#93556 0x000055555555fbaf in Rc<RefCell<CycleNode>>::eq (self=0x5555555f8cd8, other=0x5555555f8cd8)
#93557 0x000055555555fb63 in Rc<RefCell<CycleNode>>::eq (self=0x5555555f8cd8, other=0x5555555f8cd8)
#93558 0x0000555555564573 in Option<Rc<RefCell<CycleNode>>>::eq (self=0x5555555f8cd8, other=0x5555555f8cd8)
#93559 0x000055555555f864 in CycleNode::eq (self=0x5555555f8cd8, other=0x5555555f8cd8)
#93560 0x00005555555607c6 in RefCell<CycleNode>::eq (self=0x5555555f8cd0, other=0x5555555f8cd0)
#93561 0x000055555555fbaf in Rc<RefCell<CycleNode>>::eq (self=0x5555555f8ca8, other=0x5555555f8ca8)
#93562 0x000055555555fb63 in Rc<RefCell<CycleNode>>::eq (self=0x5555555f8ca8, other=0x5555555f8ca8)
#93563 0x0000555555564573 in Option<Rc<RefCell<CycleNode>>>::eq (self=0x5555555f8ca8, other=0x5555555f8ca8)
#93564 0x000055555555f864 in CycleNode::eq (self=0x5555555f8ca8, other=0x5555555f8ca8)
#93565 0x00005555555607c6 in RefCell<CycleNode>::eq (self=0x5555555f8ca0, other=0x5555555f8ca0)
#93566 0x000055555555fbaf in Rc<RefCell<CycleNode>>::eq (self=0x5555555f8c78, other=0x5555555f8c78)
#93567 0x000055555555fb63 in Rc<RefCell<CycleNode>>::eq (self=0x5555555f8c78, other=0x5555555f8c78)
#93568 0x0000555555564573 in Option<Rc<RefCell<CycleNode>>>::eq (self=0x5555555f8c78, other=0x5555555f8c78)
#93569 0x000055555555f864 in CycleNode::eq (self=0x5555555f8c78, other=0x5555555f8c78)
#93570 0x00005555555607c6 in RefCell<CycleNode>::eq (self=0x5555555f8c70, other=0x5555555f8c70)
// 1st cycle ↑
#93571 0x000055555555fbaf in Rc<RefCell<CycleNode>>::eq (self=0x7fffffffe040, other=0x7fffffffe048)
#93572 0x000055555555fb63 in Rc<RefCell<CycleNode>>::eq (self=0x7fffffffe040, other=0x7fffffffe048)
#93573 0x0000555555564573 in Option<Rc<RefCell<CycleNode>>>::eq (self=0x7fffffffe040, other=0x7fffffffe048)
#93574 0x000055555555f4ae in issue_132263::main ()

As self == other, you may want to do something like this:

impl PartialEq for CycleNode {
    fn eq(&self, other: &CycleNode) -> bool {
        (match (&self.next, &other.next) {
            (Some(x), Some(y)) => Rc::ptr_eq(x, y),
            (None, None) => true,
            _ => false,
        }) && self.pos == other.pos
    }
}

@rustbot label -C-bug -needs-triage

@rustbot rustbot removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 28, 2024
@Ayanekoji
Copy link
Author

Removing the assert_eq!(result, Some(nodes[2].clone())); solves the stack overflow; I guess the impl PartialEq for CycleNode loops infinitely ?

#93555 0x00005555555607c6 in RefCell::eq (self=0x5555555f8c70, other=0x5555555f8c70)
// 2nd cycle ↑
#93556 0x000055555555fbaf in Rc<RefCell>::eq (self=0x5555555f8cd8, other=0x5555555f8cd8)
#93557 0x000055555555fb63 in Rc<RefCell>::eq (self=0x5555555f8cd8, other=0x5555555f8cd8)
#93558 0x0000555555564573 in Option<Rc<RefCell>>::eq (self=0x5555555f8cd8, other=0x5555555f8cd8)
#93559 0x000055555555f864 in CycleNode::eq (self=0x5555555f8cd8, other=0x5555555f8cd8)
#93560 0x00005555555607c6 in RefCell::eq (self=0x5555555f8cd0, other=0x5555555f8cd0)
#93561 0x000055555555fbaf in Rc<RefCell>::eq (self=0x5555555f8ca8, other=0x5555555f8ca8)
#93562 0x000055555555fb63 in Rc<RefCell>::eq (self=0x5555555f8ca8, other=0x5555555f8ca8)
#93563 0x0000555555564573 in Option<Rc<RefCell>>::eq (self=0x5555555f8ca8, other=0x5555555f8ca8)
#93564 0x000055555555f864 in CycleNode::eq (self=0x5555555f8ca8, other=0x5555555f8ca8)
#93565 0x00005555555607c6 in RefCell::eq (self=0x5555555f8ca0, other=0x5555555f8ca0)
#93566 0x000055555555fbaf in Rc<RefCell>::eq (self=0x5555555f8c78, other=0x5555555f8c78)
#93567 0x000055555555fb63 in Rc<RefCell>::eq (self=0x5555555f8c78, other=0x5555555f8c78)
#93568 0x0000555555564573 in Option<Rc<RefCell>>::eq (self=0x5555555f8c78, other=0x5555555f8c78)
#93569 0x000055555555f864 in CycleNode::eq (self=0x5555555f8c78, other=0x5555555f8c78)
#93570 0x00005555555607c6 in RefCell::eq (self=0x5555555f8c70, other=0x5555555f8c70)
// 1st cycle ↑
#93571 0x000055555555fbaf in Rc<RefCell>::eq (self=0x7fffffffe040, other=0x7fffffffe048)
#93572 0x000055555555fb63 in Rc<RefCell>::eq (self=0x7fffffffe040, other=0x7fffffffe048)
#93573 0x0000555555564573 in Option<Rc<RefCell>>::eq (self=0x7fffffffe040, other=0x7fffffffe048)
#93574 0x000055555555f4ae in issue_132263::main ()
As self == other, you may want to do something like this:

impl PartialEq for CycleNode {
fn eq(&self, other: &CycleNode) -> bool {
(match (&self.next, &other.next) {
(Some(x), Some(y)) => Rc::ptr_eq(x, y),
(None, None) => true,
_ => false,
}) && self.pos == other.pos
}
}
@rustbot label -C-bug -needs-triage

that's very helpful and impressive, thanks!!!

@saethlin
Copy link
Member

I don't see any indication of a double free, or any other kind of bug in Rust or rustc here so I'm going to close this.

@saethlin saethlin added the C-discussion Category: Discussion or questions that doesn't represent real issues. label Oct 28, 2024
@saethlin saethlin changed the title Potential double free when using Rc<RefCell<T>> in cycle detection algorithm Stack overflow at runtime when using Rc<RefCell<T>> in cycle detection algorithm Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues.
Projects
None yet
Development

No branches or pull requests

4 participants