salsa-rs/salsa

Stack overflow on master

Closed this issue · 4 comments

Creating this issue as a discussion to flag a stack overflow issue introduced by some recent Salsa changes. The code that triggers it is pretty innocent:

#[salsa::tracked]
pub fn workflow_path_to_workspace(
	db: &dyn crate::Db,
	world: World,
	mut path: PathBuf,
) -> Workspace {
	tracing::info!("Resolving path: {:?}", path);
	let workspaces = world.workspaces(db);

	if let Some(workspace) = workspaces.get(&path) {
		tracing::info!("Workspace resolved at: {:?}", path);
		return *workspace;
	} else {
		if path.pop() {
			workflow_path_to_workspace(db, world, path)
		} else {
			panic!("Could not find workspace")
		}
	}
}

I'm seeing this function to print Workspace resolved at indicating a successful computation, but then something fails in Salsa. Calling computation never sees the result, so it should be something that is getting invoked after the computation function but before the result is returned to a calling computation (which is another tracked function in my case)

I'll try to investigate this deeper and bisect recent commits next week. I'll add more information to this issue once I have it.

That's interesting. I think a strack trace could be a very good start to narrow this down.

Is PathBuf the std::path::PathBuf or is it a salsa input or tracked struct? If it's the std::path::PathBuf, then this shouldn't compile (I think?).

@MichaReiser this is the standard std::path::PathBuf. It's pretty hard to capture stack trace when stack overflow occurs, because Rust is not unwinding the stack. I'm trying to find a way to locate where it overflows

Hmm, I don't think this query is supposed to compile unless this was an addition to more recent Salsa versions (@nikomatsakis). Could you try to avoid mutating the buffer in place?

@MichaReiser This is a false alarm and I figured it out. The stack overflow in my case was caused by the new Debug implementation for tracked structs. Previous versions of Salsa used to output them like an ID and the current one dumps the whole struct. Those structs have cyclic dependencies in my case (it's a nice benefit of Salsa), so I'm seeing a stack overflow when I try to print them.