From 878dc6c22717971f5dd4720f314f36c47a0c762e Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Sun, 20 Nov 2022 23:52:09 +0100 Subject: fix(tvix/eval): aggressively fix a borrow error in nix_eq When comparing Nix values for equality, an issue can occur where recursive values contain thunks to themselves which causes borrow errors when forcing them for comparison later down the line. To work around this we clone the values for now. There might be some optimisations possible like checking for thunk equality directly and short-circuiting on that (we have to check what Nix does). Change-Id: I7e75c992ea68f100058f52b4b46168da7d671994 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7314 Reviewed-by: sterni Tested-by: BuildkiteCI --- tvix/eval/src/value/mod.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index f0bb3628d8..ed29ae938a 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -326,13 +326,19 @@ impl Value { // Optimised attribute set comparison (Value::Attrs(a1), Value::Attrs(a2)) => Ok(Rc::ptr_eq(a1, a2) || a1.nix_eq(a2, vm)?), - // If either value is a thunk, the thunk should be forced, and then the resulting value - // must be compared instead. + // If either value is a thunk, the thunk should be forced, and then + // the resulting value must be compared instead. (Value::Thunk(lhs), Value::Thunk(rhs)) => { lhs.force(vm)?; rhs.force(vm)?; - lhs.value().nix_eq(&*rhs.value(), vm) + // TODO: this cloning is done because there is a potential issue + // with keeping borrows into both thunks around while recursing, + // as they might recurse themselves, leading to a borrow error + // when they are later being forced. + let lhs = lhs.value().clone(); + let rhs = rhs.value().clone(); + lhs.nix_eq(&rhs, vm) } (Value::Thunk(lhs), rhs) => { lhs.force(vm)?; -- cgit 1.4.1