Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Hui" == Tom Tromey <tromey@redhat.com> writes:

Hui> With this patch, GDB will output a lot of "Python not initialized"
Hui> that output by function "ensure_python_env".
Hui> For example:
Hui> (gdb) start
Hui> Temporary breakpoint 1 at 0x400507: file 2.c, line 4.
Hui> Starting program: /home/teawater/tmp/2
Hui> Python not initialized

Tom> I think this may be an independent bug.
Tom> I'll have a better patch soon.

Ok, I have looked into the problem some more.
I'm sorry this has been taking so long.

I've appended my current patch.  It arranges to clear
gdb_python_initialized if the second stage of initialization fails.
And, it adds a check on gdb_python_initialized in a couple of spots that
we missed before.

However, it still isn't quite right, due to this code in
finish_python_initialization:

  if (gdb_python_module == NULL)
    {
      gdbpy_print_stack ();
      /* This is passed in one call to warning so that blank lines aren't
	 inserted between each line of text.  */
      warning (_("\n"
		 "Could not load the Python gdb module from `%s'.\n"
		 "Limited Python support is available from the _gdb module.\n"
		 "Suggest passing --data-directory=/path/to/gdb/data-directory.\n"),
		 gdb_pythondir);
      do_cleanups (cleanup);
      return;
    }

The question is -- what should we really do here?

One option is to set gdb_python_initialized = 0.
I think this makes the warning text obsolete, though, since the
"limited" support would not actually be available at all (e.g., the
"python" command wouldn't work at all).

Another option is to try to split the initialization global.  One of
your patches in this thread did that.  It wasn't clear to me that your
patch went far enough, though -- it changed some spots to check the new
flag, but not every spot.  This approach would seem to involve auditing
all uses of gdb_python_initialized and deciding (how?) whether or not
the particular spot needs "full" initialization.

I lean toward the simpler solution of changing the warning message and
clearing the flag.

Tom

2014-01-14  Tom Tromey  <tromey@redhat.com>

	* python/py-finishbreakpoint.c (bpfinishpy_handle_stop): Check
	gdb_python_initialized.
	(bpfinishpy_handle_exit): Likewise.
	* python/python.c (_initialize_python): Don't release the GIL.
	(release_gil): New function.
	(finish_python_initialization): Use release_gil.  Don't call
	ensure_python_env.  Possibly clear gdb_python_initialized.

diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 712a9ee..4e052d7 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -382,8 +382,12 @@ bpfinishpy_detect_out_scope_cb (struct breakpoint *b, void *args)
 static void
 bpfinishpy_handle_stop (struct bpstats *bs, int print_frame)
 {
-  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
-                                               current_language);
+  struct cleanup *cleanup;
+
+  if (!gdb_python_initialized)
+    return;
+
+  cleanup = ensure_python_env (get_current_arch (), current_language);
 
   iterate_over_breakpoints (bpfinishpy_detect_out_scope_cb,
                             bs == NULL ? NULL : bs->breakpoint_at);
@@ -397,8 +401,12 @@ bpfinishpy_handle_stop (struct bpstats *bs, int print_frame)
 static void
 bpfinishpy_handle_exit (struct inferior *inf)
 {
-  struct cleanup *cleanup = ensure_python_env (target_gdbarch (),
-                                               current_language);
+  struct cleanup *cleanup;
+
+  if (!gdb_python_initialized)
+    return;
+
+  cleanup = ensure_python_env (target_gdbarch (), current_language);
 
   iterate_over_breakpoints (bpfinishpy_detect_out_scope_cb, NULL);
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 337c170..8a6389f 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1712,10 +1712,6 @@ message == an error message without a stack will be printed."),
   if (gdbpy_value_cst == NULL)
     goto fail;
 
-  /* Release the GIL while gdb runs.  */
-  PyThreadState_Swap (NULL);
-  PyEval_ReleaseLock ();
-
   make_final_cleanup (finalize_python, NULL);
 
   gdb_python_initialized = 1;
@@ -1731,6 +1727,15 @@ message == an error message without a stack will be printed."),
 
 #ifdef HAVE_PYTHON
 
+/* A cleanup function that releases the GIL.  */
+
+static void
+release_gil (void *ignore)
+{
+  PyThreadState_Swap (NULL);
+  PyEval_ReleaseLock ();
+}
+
 /* Perform the remaining python initializations.
    These must be done after GDB is at least mostly initialized.
    E.g., The "info pretty-printer" command needs the "info" prefix
@@ -1744,7 +1749,13 @@ finish_python_initialization (void)
   PyObject *sys_path;
   struct cleanup *cleanup;
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  /* If the first phase of initialization failed, don't bother doing
+     anything here.  */
+  if (!gdb_python_initialized)
+    return;
+
+  /* Release the GIL while gdb runs.  */
+  cleanup = make_cleanup (release_gil, NULL);
 
   /* Add the initial data-directory to sys.path.  */
 
@@ -1814,6 +1825,7 @@ finish_python_initialization (void)
   gdbpy_print_stack ();
   warning (_("internal error: Unhandled Python exception"));
   do_cleanups (cleanup);
+  gdb_python_initialized = 0;
 }
 
 #endif /* HAVE_PYTHON */