PY-38378 PY-38424 Fix updating variables in debugger console

We now access the `frame.f_locals` attribute only once. Multiple access leads to the loss of the changes we have made. We also alter the second argument of the `PyFrame_LocalsToFast()` to 1 to ensure that all the deleted variables will be wiped out from the frame locals.

(cherry picked from commit 28abff39b2c37f30651db220e8700032a629ce73)

GitOrigin-RevId: 8514da7ec187c1192ea9cdf851f89f008242fc30

python/helpers/pydev/_pydevd_bundle/pydevd_console_integration.py

@@ -1,4 +1,4 @@
-from _pydevd_bundle.pydevd_constants import dict_keys
+from _pydevd_bundle.pydevd_constants import dict_keys, dict_iter_items
 
 try:
     from code import InteractiveConsole
@@ -229,14 +229,18 @@ def console_exec(thread_id, frame_id, expression, dbg):
     return False
 
 
-def update_frame_local_variables_and_save(frame, var_dict):
-    """Update the frame local variables with the values from `var_dict`, remove those that no longer exist, and save."""
-    for var_name in var_dict:
-        if var_name not in frame.f_globals:
-            frame.f_locals[var_name] = var_dict[var_name]
-        elif var_dict[var_name] is not frame.f_globals[var_name]:
-            frame.f_locals[var_name] = var_dict[var_name]
-    for var_name in frame.f_locals:
-        if var_name not in var_dict:
-            frame.f_locals.pop(var_name)
+def update_frame_local_variables_and_save(frame, values):
+    """Update the frame local variables with the values from `values`, remove those that no longer exist, and save."""
+    # It is important to access to `frame.f_locals` through a local reference.
+    # Any changes done to it will be lost as soon as we access the attribute next time.
+    f_locals, f_globals = frame.f_locals, frame.f_globals
+
+    for key, value in dict_iter_items(values):
+        if key not in f_globals or value is not f_globals[key]:
+            f_locals[key] = value
+
+    for key in dict_keys(f_locals):
+        if key not in values:
+            f_locals.pop(key)
+
     pydevd_save_locals.save_locals(frame)

python/helpers/pydev/_pydevd_bundle/pydevd_save_locals.py

@@ -58,7 +58,7 @@ def make_save_locals_impl():
         pass
     else:
         def save_locals_ctypes_impl(frame):
-            locals_to_fast(ctypes.py_object(frame), ctypes.c_int(0))
+            locals_to_fast(ctypes.py_object(frame), ctypes.c_int(1))
 
         return save_locals_ctypes_impl
 

python/testData/debug/test_update_series_in_debug_console.py

@@ -0,0 +1,15 @@
+import pandas as pd
+
+
+class C:
+    @staticmethod
+    def f():
+
+        def g():
+            return pd.Series(index=[40, 50, 60], data=[1, 2, 3])
+
+        a = g()
+        a  # breakpoint
+
+
+C.f()

python/testSrc/com/jetbrains/env/python/debug/PyBaseDebuggerTask.java

@@ -418,8 +418,14 @@ public abstract class PyBaseDebuggerTask extends PyExecutionFixtureTestTask {
     return result.first;
   }
 
-  protected void consoleExec(String command) throws PyDebuggerException {
-    // We can't wait for result with a callback, because console just prints it to output
+  /**
+   * Run a command in the debugger console without waiting for the result.
+   *
+   * @param command to run.
+   *
+   * @see #consoleExecAndWait(String)
+   */
+  protected void consoleExec(String command) {
     myDebugProcess.consoleExec(command, new PyDebugCallback<String>() {
       @Override
       public void ok(String value) {
@@ -431,7 +437,30 @@ public abstract class PyBaseDebuggerTask extends PyExecutionFixtureTestTask {
     });
   }
 
-  protected Variable eval(String name) throws InterruptedException {
+  /**
+   * Run a command in the debugger console and wait until it is executed. It raises the assertion error if the command
+   * hasn't finished withing {@link XDebuggerTestUtil#TIMEOUT_MS} milliseconds. It doesn't matter if the command itself
+   * has finished successfully or failed (e.g. incorrect commands can be used in tests in purpose).
+   *
+   * @param command to run.
+   */
+  protected void consoleExecAndWait(String command) {
+    EvaluationCallback<String> callback = new EvaluationCallback<>();
+    myDebugProcess.consoleExec(command, new PyDebugCallback<String>() {
+      @Override
+      public void ok(String value) {
+        callback.evaluated(value);
+      }
+
+      @Override
+      public void error(PyDebuggerException exception) {
+        callback.errorOccurred(exception.getMessage());
+      }
+    });
+    callback.waitFor(XDebuggerTestUtil.TIMEOUT_MS);
+  }
+
+  protected Variable eval(String name) {
     Assert.assertTrue("Eval works only while suspended", mySession.isSuspended());
     XValue var = XDebuggerTestUtil.evaluate(mySession, name).first;
     Assert.assertNotNull("There is no variable named " + name, var);

python/testSrc/com/jetbrains/env/python/debug/PyDebuggerTask.java

@@ -289,4 +289,28 @@ public class PyDebuggerTask extends PyBaseDebuggerTask {
       myDebugProcess.getProcessHandler().destroyProcess();
     }
   }
+
+  /**
+   * Toggles breakpoint in the script returned by {@link PyDebuggerTask#getScriptName()}.
+   *
+   * @param line starting with 0
+   */
+  protected void toggleBreakpoint(int line) {
+    toggleBreakpoint(getFilePath(getScriptName()), line);
+  }
+
+  /**
+   * Toggles multiple breakpoints with {@link PyDebuggerTask#toggleBreakpoint(int)}.
+   */
+  protected void toggleBreakpoints(int... lines) {
+    toggleBreakpoints(getFilePath(getScriptName()), lines);
+  }
+
+  /**
+   * Toggles multiple breakpoints with {@link PyDebuggerTask#toggleBreakpoint(String, int)}.
+   */
+  protected void toggleBreakpoints(@NotNull String file, int... lines) {
+    for(int line : lines)
+      toggleBreakpoint(file, line);
+  }
 }

python/testSrc/com/jetbrains/env/python/debug/PythonDebugConsoleTest.java

@@ -0,0 +1,91 @@
+// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package com.jetbrains.env.python.debug;
+
+import com.google.common.collect.ImmutableSet;
+import com.jetbrains.env.PyEnvTestCase;
+import org.jetbrains.annotations.NotNull;
+import org.junit.Test;
+
+import java.util.Set;
+
+public class PythonDebugConsoleTest extends PyEnvTestCase {
+
+  @Test
+  public void testDebugConsoleExecAndOutput() {
+    runPythonTest(new PyDebuggerTask("/debug", "test1.py") {
+      @Override
+      public void before() {
+        toggleBreakpoint(getFilePath(getScriptName()), 3);
+        setWaitForTermination(false);
+      }
+
+      @Override
+      public void testing() throws Exception {
+        waitForPause();
+        eval("i").hasValue("0");
+        resume();
+        waitForPause();
+        consoleExec("'i=%d'%i");
+        waitForOutput("'i=1'");
+        consoleExec("x");
+        waitForOutput("name 'x' is not defined");
+        consoleExec("1-;");
+        waitForOutput("SyntaxError");
+        resume();
+      }
+    });
+  }
+
+  // PY-38378
+  @Test
+  public void testUpdateVariableInDebugConsole() {
+    runPythonTest(new PyDebuggerTask("/debug", "test2.py") {
+      @Override
+      public void before() {
+        toggleBreakpoints(5, 7);
+      }
+
+      @Override
+      public void testing() throws Exception {
+        waitForPause();
+        consoleExecAndWait("z = 42");
+        resume();
+        waitForPause();
+        eval("z").hasValue("43");
+        resume();
+        waitForTerminate();
+      }
+    });
+  }
+
+  // PY-38424
+  @Test
+  public void testUpdateSeriesInDebugConsole() {
+    runPythonTest(new PyDebuggerTask("/debug", "test_update_series_in_debug_console.py") {
+      @Override
+      public void before() throws Exception {
+        toggleBreakpoint(11);
+      }
+
+      @Override
+      public void testing() throws Exception {
+        waitForPause();
+        eval("str(a[40])").hasValue("'1'");
+        consoleExecAndWait("a[40] = 100");
+        eval("str(a[40])").hasValue("'100'");
+        consoleExecAndWait("        def g():\n" +
+                    "            return pd.Series(index=[40,50,60], data=[1,2,3])");
+        consoleExecAndWait("a = g()");
+        eval("str(a[40])").hasValue("'1'");
+        resume();
+        waitForTerminate();
+      }
+
+      @NotNull
+      @Override
+      public Set<String> getTags() {
+        return ImmutableSet.of("pandas");
+      }
+    });
+  }
+}

python/testSrc/com/jetbrains/env/python/debug/PythonDebuggerTest.java

@@ -108,42 +108,6 @@ public class PythonDebuggerTest extends PyEnvTestCase {
     });
   }
 
-  @Test
-  public void testDebugConsole() {
-    runPythonTest(new PyDebuggerTask("/debug", "test1.py") {
-      @Override
-      public void before() {
-        toggleBreakpoint(getFilePath(getScriptName()), 3);
-        setWaitForTermination(false);
-      }
-
-      @Override
-      public void testing() throws Exception {
-        waitForPause();
-
-        eval("i").hasValue("0");
-
-        resume();
-
-        waitForPause();
-
-        consoleExec("'i=%d'%i");
-
-        waitForOutput("'i=1'");
-
-        consoleExec("x");
-
-        waitForOutput("name 'x' is not defined");
-
-        consoleExec("1-;");
-
-        waitForOutput("SyntaxError");
-
-        resume();
-      }
-    });
-  }
-
   @Test
   public void testDebugCompletion() {
     runPythonTest(new PyDebuggerTask("/debug", "test4.py") {