rstudio/rstudio — pull request #163 by jmcphers (2014-09-12 21:23:55+00:00)
Title: Fix runaway recursion resulting from Step Into

This change fixes a problem reported by @tareefk in which stepping into a call while debugging at the top level can cause a runaway recursion tail that eventually exhausts the stack and crashes the session.

The problem is in the way that R handles the Step Into command. It has a global variable `R_BrowserLastCommand` that stores the last command entered at the `Browse[N]> ` prompt. When this variable is set to `'s'`, the next function executed--*no matter what it is*--will be debugged.

There are operations we perform on every console prompt (including building the environment pane if necessary) that can trigger R code to run. When that code runs, it runs R functions, and because `R_BrowserLastCommand` is still `s`, those functions get debugged. When they're debugged we get a new debug prompt, which makes the functions run again, and so on, and so forth.

We let the R Core team know about this bug a few months ago (see [R-Project Bug 15770](https://bugs.r-project.org/bugzilla/show_bug.cgi?id=15770)), and they fixed it: https://github.com/wch/r-source/commit/8b672fe4f178aac0d2bea64097befb36b9f824f3. Unfortunately the fix applies to functions parsed and executed at the console, not (always) to functions that are invoked via the C API that RStudio calls.

The fix here works with (and/or mimics) the fix made by the R Core team. Before we execute R functions, we check to see if the last browser command executed was `'s'`. If it is, we change it temporarily while we execute the function, and restore it when we're done. It's similar in principle and design to the mechanism we use to suppress error handlers while we run RStudio internal code.

Two other small notes:
- This problem existed in R 3.1, but seems to be easier to hit now (possibly new changes in R 3.1.1).
- Any part of the code where we evaluate R statements or execute R expressions at a time when the user could be debugging needs to be guarded. This is a lot of surface area. I've covered the major chokepoints but it might be worth a backlog item to look for others.