Date: Sun, 19 Feb 2017 11:31:44 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Hans Petter Selasky <hps@selasky.org> Cc: FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: First thread in proc in not passed to thread_dtor eventhandler upon exit Message-ID: <20170219093144.GU2092@kib.kiev.ua> In-Reply-To: <060681c7-4c46-dcc7-d797-868fd0ad35a6@selasky.org> References: <933e132d-289b-330d-b349-584a25e279d4@selasky.org> <20170219035422.GQ2092@kib.kiev.ua> <060681c7-4c46-dcc7-d797-868fd0ad35a6@selasky.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Feb 19, 2017 at 09:43:24AM +0100, Hans Petter Selasky wrote:
> On 02/19/17 04:54, Konstantin Belousov wrote:
> > On Sat, Feb 18, 2017 at 10:40:00PM +0100, Hans Petter Selasky wrote:
> >> Hi,
> >>
> >> Is the following a bug or feature. I observe that the first thread in a
> >> procedure is not passed to thread_dtor as declared by the following
> >> eventhandler, when the procedure exits.
> >>
> >> EVENTHANDLER_DECLARE(thread_dtor, thread_dtor_fn);
> >>
> >> Is this a bug or feature?
> > This is a feature. When a zombie process is reaped, the last thread in
> > the process (the one which exited it) is left in the process thread list.
> > This is an optimization, because process without at least one thread
> > is never useful.
> >
> > You can see the code in fork1() which allocates struct proc from zone
> > and then checks if there any thread pre-allocated as well (both struct
> > proc and struct thread are type-stable).
> >
> > Since the last thread is not freed, its destructor is not signalled.
>
> Hi Konstantin,
>
> I see. To avoid leakage of LinuxKPI, Linux and Dtrace resources to all
> system threads, I think the attached patch is required. What do you
> think about the attached patch? If you don't like it can you suggest
> another approach?
Your patch is in-line with e.g. osd handling, so I do not see a reason
to suggest a different approach. Looks fine.
Of course, the only worry is the fact that event handlers are called
for the thread which is not newly allocated nor just freed.
> diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
> index f39326f..a206a0a 100644
> --- a/sys/kern/kern_proc.c
> +++ b/sys/kern/kern_proc.c
> @@ -191,11 +191,17 @@ static int
> proc_ctor(void *mem, int size, void *arg, int flags)
> {
> struct proc *p;
> + struct thread *td;
>
> p = (struct proc *)mem;
> SDT_PROBE4(proc, , ctor , entry, p, size, arg, flags);
> EVENTHANDLER_INVOKE(process_ctor, p);
> SDT_PROBE4(proc, , ctor , return, p, size, arg, flags);
> + td = FIRST_THREAD_IN_PROC(p);
> + if (td != NULL) {
> + /* Make sure all thread constructors are executed */
> + EVENTHANDLER_INVOKE(thread_ctor, td);
> + }
> return (0);
> }
>
> @@ -220,6 +226,9 @@ proc_dtor(void *mem, int size, void *arg)
> #endif
> /* Free all OSD associated to this thread. */
> osd_thread_exit(td);
> +
> + /* Make sure all thread destructors are executed */
> + EVENTHANDLER_INVOKE(thread_dtor, td);
> }
> EVENTHANDLER_INVOKE(process_dtor, p);
> if (p->p_ksi != NULL)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170219093144.GU2092>