Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: gdb-patches@sourceware.org
Cc: Joel Brobecker <brobecker@adacore.com>
Subject: [RFA/commit/procfs] /proc/.../map file descriptor leak
Date: Wed, 09 Nov 2011 18:47:00 -0000	[thread overview]
Message-ID: <1320864439-12456-1-git-send-email-brobecker@adacore.com> (raw)

Hello,

This is on sparc-solaris.  I was trying to reproduce another error
that our testsuite occasionally reports (without much success :-().

I noticed while running the test repeatedly within the same debugger
process (that is: repeatedly doing "run; [...]; kill" instead of
starting a new debugger every time) that it would eventually cause
another error in the debugger. The debugger would be telling us that
it is unable to find a given thread in the procinfo list. In other
words, the debugger received an event for LWP=1 and then failed
to create the assocated thread element in the procinfo list.  After some
investgation, it turned out that this was because we were unable to open
the associated lwp file in the /proc filesystem, and that was because we
ran out of file descriptors!

Digging further, I found that we leak the file descriptor created
when opening the procfs map file. we create a cleanup routine to make
sure that the associated file descriptor gets closed, but we never
call the cleanup.

gdb/ChangeLog:

        * procfs.c (iterate_over_mappings): Call do_cleanups before
        returning.

Fixed thusly. Seems fairly straightforward, except that the cleanup
is only really needed when NEW_PROC_API is defined (basically, Solaris
except old versions).  But I think that #ifdef code is ugly, so I made
the cleanup run regardless...

I'll commit in a couple of days pending feedback...

---
 gdb/procfs.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/gdb/procfs.c b/gdb/procfs.c
index 871dd47..2a253a1 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -5217,6 +5217,7 @@ iterate_over_mappings (procinfo *pi, find_memory_region_ftype child_func,
   int funcstat;
   int map_fd;
   int nmap;
+  struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
 #ifdef NEW_PROC_API
   struct stat sbuf;
 #endif
@@ -5254,8 +5255,12 @@ iterate_over_mappings (procinfo *pi, find_memory_region_ftype child_func,
 
   for (prmap = prmaps; nmap > 0; prmap++, nmap--)
     if ((funcstat = (*func) (prmap, child_func, data)) != 0)
-      return funcstat;
+      {
+	do_cleanups (cleanups);
+        return funcstat;
+      }
 
+  do_cleanups (cleanups);
   return 0;
 }
 
-- 
1.7.1


             reply	other threads:[~2011-11-09 18:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-09 18:47 Joel Brobecker [this message]
2011-11-09 19:06 ` Tom Tromey
2011-11-09 19:46   ` Joel Brobecker
2011-11-09 19:52     ` Tom Tromey
2011-11-09 19:58       ` Joel Brobecker
2011-11-09 20:00         ` Tom Tromey
2011-11-10 16:37     ` Joel Brobecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1320864439-12456-1-git-send-email-brobecker@adacore.com \
    --to=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox