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

[Hui Zhu <hui_zhu at mentor dot com>]

On 12/12/13 00:30, Tom Tromey wrote:
> > > > > > "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:
>
> > > Two other approaches are possible here instead.  One, change
> > > finish_python_initialization to do the needed bit of locking by handy,
> > > not using ensure_python_env.  Or, two, don't release the GIL until
> > > somewhere in finish_python_initialization, and then it doesn't need
> > > to call ensure_python_env at all.
>
> Hui> I think the first way is better than second way because
> Hui> ensure_python_env has relationship with current_arch and
> Hui> current_language.  So I make a patch according the first way.
>
> After seeing the patch I think it is probably preferable to do the
> second route -- move the GIL releasing to finish_python_initialization.
>
> Can you try the appended?

Hi Tom,

With this patch, GDB will output a lot of "Python not initialized" that output by function "ensure_python_env".
For example:
(gdb) start
Temporary breakpoint 1 at 0x400507: file 2.c, line 4.
Starting program: /home/teawater/tmp/2
Python not initialized
(gdb) bt
#0  main (argc=1, argv=0x7fffffffe138, envp=0x7fffffffe148) at 2.c:4
(gdb) quit
A debugging session is active.

	Inferior 1 [process 31052] will be killed.

Quit anyway? (y or n) y
Python not initialized

Thanks,
Hui

> Tom
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 35a1d73..6554be2 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,11 @@
> +2013-12-11  Tom Tromey  <tromey@redhat.com>
> +
> +	* python/python.c (_initialize_python): Don't release the GIL or
> +	set gdb_python_initialized.
> +	(release_gil): New function.
> +	(finish_python_initialization): Use release_gil.  Don't call
> +	ensure_python_env.  Set gdb_python_initialized.
> +
>   2013-12-11  Sergio Durigan Junior  <sergiodj@redhat.com>
>
>   	* break-catch-throw.c (fetch_probe_arguments): Pass selected frame
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 1873936..ddf8a1a 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -1711,18 +1711,12 @@ 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;
>     return;
>
>    fail:
>     gdbpy_print_stack ();
> -  /* Do not set 'gdb_python_initialized'.  */
>     return;
>
>   #endif /* HAVE_PYTHON */
> @@ -1730,6 +1724,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
> @@ -1743,7 +1746,8 @@ finish_python_initialization (void)
>     PyObject *sys_path;
>     struct cleanup *cleanup;
>
> -  cleanup = ensure_python_env (get_current_arch (), current_language);
> +  /* Release the GIL while gdb runs.  */
> +  cleanup = make_cleanup (release_gil, NULL);
>
>     /* Add the initial data-directory to sys.path.  */
>
> @@ -1807,12 +1811,16 @@ finish_python_initialization (void)
>        variable.  */
>
>     do_cleanups (cleanup);
> +
> +  gdb_python_initialized = 1;
> +
>     return;
>
>    fail:
>     gdbpy_print_stack ();
>     warning (_("internal error: Unhandled Python exception"));
>     do_cleanups (cleanup);
> +  /* Do not set 'gdb_python_initialized'.  */
>   }
>
>   #endif /* HAVE_PYTHON */