This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 3/3] Refactor atfork handlers

On 02/08/2018 01:50 PM, Adhemerval Zanella wrote:
+static struct fork_handler *
+fork_handler_list_find_if (struct fork_handler_list *fork_handlers,
+			   void *dso_handle)

Should be called _find, not find_if (no callback is involved).

+  struct fork_handler *first = fork_handler_list_find_if (&fork_handlers,
+							  dso_handle);
+  /* Removing is done by shifting the elements in the way the elements
+     that are not to be removed appear in the beginning in dynarray.
+     This avoid the quadradic run-time if a naive strategy to remove and
+     shift one element at time.  */
+  if (first != NULL)
+    {
+      struct fork_handler *result = first;

result should probably be called new_end or something like that.

+      first++;
+      for (; first != fork_handler_list_end (&fork_handlers); ++first)
+	{
+	  if (first->dso_handle != dso_handle)
+	    {
+	      memcpy (result, first, sizeof (struct fork_handler));

Wouldn't a simple struct assignment work here?

I think this patch is a step in the right direction, so it should go in with these changes.

However, I think we should make a few improvements in follow-up fixes:

Reduce RSS usage for the common case that no atfork handlers are ever registered. This will be the case once we remove the bogus __reclaim_stacks function.

Make a temporary copy of the handler array during fork. This has two benefits: We can run the handlers without acquiring the handler lock (to avoid application deadlocks). We also make sure that a handler does not run in a child process which did not run in the parent process. I think the old implementation had both properties.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]