This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Checkpoint: wait the defunct process when delete it
On Thu, May 13, 2010 at 18:30, Pedro Alves <pedro@codesourcery.com> wrote:
>
> On Thursday 13 May 2010 08:58:49, Hui Zhu wrote:
> > >> 2. ?before call "inferior_call_waitpid" to waitpid the ptid.
> > >> Check if ppid is a simple thread. ?ppid > 1
> > >> Check if ppid is the GDB. ?If ppid is GDB, it will auto wait the ptid.
> > >
> > > What do you mean, it will auto wait the ptid? ?AFAICS,
> > >
> > > (gdb) checkpoint
> > > (gdb) checkpoint
> > > (gdb) checkpoint
> > > (gdb) checkpoint
> > > (gdb) checkpoint
> > > (gdb) restart 5
> > > (gdb) delete checkpoint 0
> > >
> > > will still leave checkpoint 0 zombie?
> >
> > This is because the parent of checkpoint is GDB. ?GDB will auto wait
> > the zombie, so I just leave them there let GDB hanle it.
>
> You didn't answer the question. ?Please point me at where is
> this "gdb auto waiting".
>
> I actually don't understand that rationale. ?The `init' process
> will "auto wait" ?the checkpoint forks as well, so why bother in
> the first place then?
>
my_waitpid with -1 will handle them.
And looks if we don't call getppid, we will not have a good way to make sure if
> > + ?if ((!find_thread_ptid (fi->parent_ptid) && find_fork_ptid (fi->parent_ptid))
> > + ? ? ?|| (find_thread_ptid (fi->parent_ptid) && is_stopped (fi->parent_ptid)))
>
> This requires an explaning comment in the code.
OK. I will add them.
>
> > >> +
> > >> + ?ret = call_function_by_hand (getppid_fn, 0, NULL);
> > >> + ?if (ret == 0)
> > >> + ? ?return ppid;
> > >
> > > ??? can getppid really return 0 ?
> >
> > This 0 is not the return value of getppid.
>
> Oh, right. ?:-)
>
> > This is how function "checkpoint_command" use this function. ?Check it
> > just for code safe. ?:)
> > ? ret = call_function_by_hand (fork_fn, 0, &ret);
> > ? do_cleanups (old_chain);
> > ? if (!ret) ? ? /* Probably can't happen. ?*/
> > ? ? error (_("checkpoint: call_function_by_hand returned null."));
>
> Well, the only return from call_function_by_hand does:
>
> ? ?gdb_assert (retval);
> ? ?return retval;
>
> All other cases are handled by throwing an error. ? So
> "definitely can't happen"; please remove the new dead code
> you're adding.
Thanks. I will fix it.
>
> BTW, you patch is not error/exception safe. ?You didn't consider
> the case of an `error' being thrown between switching the
> fork and back.
Thanks for alarm me about it. I have add a
inferior_call_waitpid_cleanup to clean up And move
inferior_call_waitpid to the end of function
delete_checkpoint_command. Then it will not affect inferior_ptid and
delete_fork (ptid);
>
> Otherwise, the patch looks good.
Thanks. I fixed them all and checked in.
Best regards,
Hui
2010-05-14 Hui Zhu <teawater@gmail.com>
Michael Snyder <msnyder@vmware.com>
* linux-fork.c (gdbthread.h): New include.
(fork_info): Add parent_ptid.
(inferior_call_waitpid_cleanup, inferior_call_waitpid): New
functions.
(delete_checkpoint_command): Call inferior_call_waitpid.
(checkpoint_command): Set parent_ptid.
---
linux-fork.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
--- a/linux-fork.c
+++ b/linux-fork.c
@@ -29,6 +29,7 @@
#include "gdb_string.h"
#include "linux-fork.h"
#include "linux-nat.h"
+#include "gdbthread.h"
#include <sys/ptrace.h>
#include "gdb_wait.h"
@@ -47,6 +48,7 @@ struct fork_info
{
struct fork_info *next;
ptid_t ptid;
+ ptid_t parent_ptid;
int num; /* Convenient handle (GDB fork id) */
struct regcache *savedregs; /* Convenient for info fork, saves
having to actually switch contexts. */
@@ -410,12 +412,74 @@ linux_fork_detach (char *args, int from_
delete_fork (inferior_ptid);
}
+static void
+inferior_call_waitpid_cleanup (void *fp)
+{
+ struct fork_info *oldfp = fp;
+
+ /* Switch back to inferior_ptid. */
+ remove_breakpoints ();
+ fork_load_infrun_state (oldfp);
+ insert_breakpoints ();
+}
+
+static int
+inferior_call_waitpid (ptid_t pptid, int pid)
+{
+ struct objfile *waitpid_objf;
+ struct value *waitpid_fn = NULL;
+ struct value *argv[4];
+ struct gdbarch *gdbarch = get_current_arch ();
+ struct fork_info *oldfp = NULL, *newfp = NULL;
+ struct cleanup *old_cleanup = NULL;
+ int ret = -1;
+
+ if (!ptid_equal (pptid, inferior_ptid))
+ {
+ /* Switch to pptid. */
+ oldfp = find_fork_ptid (inferior_ptid);
+ gdb_assert (oldfp != NULL);
+ newfp = find_fork_ptid (pptid);
+ gdb_assert (oldfp != NULL);
+ fork_save_infrun_state (oldfp, 1);
+ remove_breakpoints ();
+ fork_load_infrun_state (newfp);
+ insert_breakpoints ();
+
+ old_cleanup = make_cleanup (inferior_call_waitpid_cleanup, oldfp);
+ }
+
+ /* Get the waitpid_fn. */
+ if (lookup_minimal_symbol ("waitpid", NULL, NULL) != NULL)
+ waitpid_fn = find_function_in_inferior ("waitpid", &waitpid_objf);
+ if (!waitpid_fn && lookup_minimal_symbol ("_waitpid", NULL, NULL) != NULL)
+ waitpid_fn = find_function_in_inferior ("_waitpid", &waitpid_objf);
+ if (!waitpid_fn)
+ goto out;
+
+ /* Get the argv. */
+ argv[0] = value_from_longest (builtin_type (gdbarch)->builtin_int, pid);
+ argv[1] = value_from_pointer (builtin_type (gdbarch)->builtin_data_ptr, 0);
+ argv[2] = value_from_longest (builtin_type (gdbarch)->builtin_int, 0);
+ argv[3] = 0;
+
+ call_function_by_hand (waitpid_fn, 3, argv);
+
+ ret = 0;
+
+out:
+ if (old_cleanup)
+ do_cleanups (old_cleanup);
+ return ret;
+}
+
/* Fork list <-> user interface. */
static void
delete_checkpoint_command (char *args, int from_tty)
{
ptid_t ptid;
+ struct fork_info *fi;
if (!args || !*args)
error (_("Requires argument (checkpoint id to delete)"));
@@ -431,10 +495,24 @@ Please switch to another checkpoint befo
if (ptrace (PTRACE_KILL, PIDGET (ptid), 0, 0))
error (_("Unable to kill pid %s"), target_pid_to_str (ptid));
+ fi = find_fork_ptid (ptid);
+ gdb_assert (fi);
+
if (from_tty)
printf_filtered (_("Killed %s\n"), target_pid_to_str (ptid));
delete_fork (ptid);
+
+ /* If fi->parent_ptid is not a part of lwp but it's a part of checkpoint
+ list, waitpid the ptid.
+ If fi->parent_ptid is a part of lwp and it is stoped, waitpid the
+ ptid. */
+ if ((!find_thread_ptid (fi->parent_ptid) && find_fork_ptid (fi->parent_ptid))
+ || (find_thread_ptid (fi->parent_ptid) && is_stopped (fi->parent_ptid)))
+ {
+ if (inferior_call_waitpid (fi->parent_ptid, PIDGET (ptid)))
+ warning (_("Unable to wait pid %s"), target_pid_to_str (ptid));
+ }
}
static void
@@ -596,6 +674,7 @@ checkpoint_command (char *args, int from
if (!fp)
error (_("Failed to find new fork"));
fork_save_infrun_state (fp, 1);
+ fp->parent_ptid = last_target_ptid;
}
static void