BlockstreamResearch / rust-simplicity

Creative Commons Zero v1.0 Universal
58 stars 12 forks source link

Deadlock in type inference #224

Closed apoelstra closed 2 months ago

apoelstra commented 2 months ago

If you add the following unit test to src/types/mod.rs it will deadlock while printing:

    #[test]
    fn memory_leak() {
        let iden = Arc::<WitnessNode<Core>>::iden();
        let drop = Arc::<WitnessNode<Core>>::drop_(&iden);
        let case = Arc::<WitnessNode<Core>>::case(&iden, &drop).unwrap();

        println!("{:?}", case.arrow());
    }

Occurs even with #221 and #222 . The issue here is not that the types are logically infinite (though I think they are) but that we have a actual refcount cycle in our Arc<Mutex>es. This leads to a deadlock in BoundMutex::get.

If you don't try to print the types, the test will complete, but it will leak memory, which can be seen by running the test in valgrind.

Investigating.

apoelstra commented 2 months ago

On further inspection I think the deadlock bug is in the #derived Debug impl on Type.

And the memory leak is unrelated and I'll file a new bug once I have nailed down the deadlock..

apoelstra commented 2 months ago

Yep, cool, I fixed the deadlock (we have this nice mod bound_mutex which prevents deadlocks by limiting the inner field of BoundMutex to that tiny module which clearly only accesses inner from set and get, neither of which hold the mutex open .... so a deadlock is impossible, right? No! There is a hidden access to inner in the #[derive(Debug)] line. Very sneaky but easy to fix.).

But this has unveiled a different, weirder deadlock. Still digging..

apoelstra commented 2 months ago

Never mind, the other deadlock is just the one that is fixed in #221. It looked "weird" because it wasn't actually a deadlock, it was just an infinite loop.

I have a fix for this but I would like to get #221 and #222 in first.