Skip to content

Conversation

@lionel-
Copy link
Contributor

@lionel- lionel- commented Dec 16, 2025

Addresses posit-dev/positron#5024

I can't reproduce the deadlock anymore, but the approach taken here should fix all deadlock issues: We only take the lock on the DAP state when we're on the R thread.

@lionel- lionel- requested a review from DavisVaughan December 16, 2025 08:43
@DavisVaughan
Copy link
Contributor

I managed to deadlock it in dplyr with dev ark, i.e. cargo build without --release.

# in dplyr
devtools::load_all()
debugonce(mutate_col)
mutate(mtcars, x = mpg+cyl)

I stepped in and out of some functions and it deadlocked while coming out of eval_all_mutate()

I think I can confirm your fix here based on the frozen stack in ark:

  • the main R thread is waiting on the lock in debug_start()
  • but the ark-dap thread is indeed paused in collect_r_variables() waiting on the r_task() to run while its holding the lock on state
  • so the r_task() can't run because the main R thread can't advance, etc, etc, deadlock

You'll also see a request for variables was just received in the logs, but then we got stuck

Screenshot 2025-12-16 at 10 56 42 AM Screenshot 2025-12-16 at 10 55 53 AM Screenshot 2025-12-16 at 10 54 30 AM

Copy link
Contributor

@DavisVaughan DavisVaughan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't seem to deadlock it with this PR, though it took me a long time to do it with dplyr the first time around so I can't be 100% certain. But based on my screenshots above, this looks like the right fix!

};
// Wait until we're in the `r_task()` to lock
// See https://github.com/posit-dev/positron/issues/5024
let state = self.state.clone();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let state = self.state.clone();
let state = Arc::clone(&self.state);

Just so it's very clear this is a cheap clone. I had to go look it up myself to make sure.

This is a typical rust pattern i see with Arc

@lionel-
Copy link
Contributor Author

lionel- commented Dec 16, 2025

yep same diagnostics as ^^ was provided in posit-dev/positron#5024, it's good to confirm we're observing the same thing (or was, I can't reproduce anymore)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants