[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Emacs-diffs] trunk r113384: Fix races with threads and file descriptors
From: |
Paul Eggert |
Subject: |
[Emacs-diffs] trunk r113384: Fix races with threads and file descriptors. |
Date: |
Fri, 12 Jul 2013 02:03:52 +0000 |
User-agent: |
Bazaar (2.6b2) |
------------------------------------------------------------
revno: 113384
revision-id: address@hidden
parent: address@hidden
committer: Paul Eggert <address@hidden>
branch nick: trunk
timestamp: Thu 2013-07-11 19:03:47 -0700
message:
Fix races with threads and file descriptors.
* configure.ac (PTY_TTY_NAME_SPRINTF): Use emacs_close, not close.
* src/callproc.c (Fcall_process_region):
* src/dired.c (open_directory):
* src/emacs.c (main, Fdaemon_initialized):
* src/image.c (x_find_image_file):
* src/inotify.c (Finotify_rm_watch):
* src/lread.c (Flocate_file_internal):
* src/process.c (Fnetwork_interface_list, Fnetwork_interface_info):
* src/term.c (term_mouse_moveto, init_tty):
* src/termcap.c (tgetent):
* src/unexaix.c, src/unexcoff.c (report_error, report_error_1,
adjust_lnnoptrs)
* src/unexaix.c, src/unexcoff.c, src/unexcw.c, src/unexelf.c (unexec):
* src/unexhp9k800.c, src/unexmacosx.c (unexec):
* src/callproc.c (Fcall_process_region):
Use emacs_close, not close.
* src/sysdep.c (POSIX_CLOSE_RESTART, posix_close) [!POSIX_CLOSE_RESTART]:
New macro and function, which emulates the POSIX_CLOSE_RESTART macro
and posix_close function on current platforms (which all lack them).
(emacs_close): Use it. This should fix the races on GNU/Linux and
on AIX and on future platforms that support POSIX_CLOSE_RESTART,
and it should avoid closing random victim file descriptors on
other platforms.
modified:
ChangeLog changelog-20091113204419-o5vbwnq5f7feedwu-1538
configure.ac
configure.in-20091113204419-o5vbwnq5f7feedwu-783
src/ChangeLog changelog-20091113204419-o5vbwnq5f7feedwu-1438
src/callproc.c callproc.c-20091113204419-o5vbwnq5f7feedwu-248
src/dired.c dired.c-20091113204419-o5vbwnq5f7feedwu-171
src/emacs.c emacs.c-20091113204419-o5vbwnq5f7feedwu-241
src/image.c image.c-20091113204419-o5vbwnq5f7feedwu-2969
src/inotify.c inotify.c-20121210111500-2cd7np2oq1r0lmp9-1
src/lread.c lread.c-20091113204419-o5vbwnq5f7feedwu-266
src/process.c process.c-20091113204419-o5vbwnq5f7feedwu-462
src/sysdep.c sysdep.c-20091113204419-o5vbwnq5f7feedwu-448
src/term.c term.c-20091113204419-o5vbwnq5f7feedwu-220
src/termcap.c termcap.c-20091113204419-o5vbwnq5f7feedwu-775
src/unexaix.c unexaix.c-20091113204419-o5vbwnq5f7feedwu-147
src/unexcoff.c unexec.c-20091113204419-o5vbwnq5f7feedwu-184
src/unexcw.c unexcw.c-20091113204419-o5vbwnq5f7feedwu-3010
src/unexelf.c unexelf.c-20091113204419-o5vbwnq5f7feedwu-426
src/unexhp9k800.c
unexhp9k800.c-20091113204419-o5vbwnq5f7feedwu-4481
src/unexmacosx.c
unexmacosx.c-20091113204419-o5vbwnq5f7feedwu-2413
=== modified file 'ChangeLog'
--- a/ChangeLog 2013-07-11 17:18:48 +0000
+++ b/ChangeLog 2013-07-12 02:03:47 +0000
@@ -1,3 +1,8 @@
+2013-07-12 Paul Eggert <address@hidden>
+
+ Fix races with threads and file descriptors.
+ * configure.ac (PTY_TTY_NAME_SPRINTF): Use emacs_close, not close.
+
2013-07-10 Paul Eggert <address@hidden>
* Makefile.in (removenullpaths): Remove adjacent null paths (Bug#14835).
=== modified file 'configure.ac'
--- a/configure.ac 2013-07-09 18:06:25 +0000
+++ b/configure.ac 2013-07-12 02:03:47 +0000
@@ -3931,7 +3931,7 @@
AC_DEFINE(PTY_ITERATION, [int i; for (i = 0; i < 1; i++)])
dnl Note that grantpt and unlockpt may fork. We must block SIGCHLD
dnl to prevent sigchld_handler from intercepting the child's death.
- AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptyname = 0; sigset_t blocked;
sigemptyset (&blocked); sigaddset (&blocked, SIGCHLD); pthread_sigmask
(SIG_BLOCK, &blocked, 0); if (grantpt (fd) != -1 && unlockpt (fd) != -1)
ptyname = ptsname(fd); pthread_sigmask (SIG_UNBLOCK, &blocked, 0); if
(!ptyname) { close (fd); return -1; } snprintf (pty_name, sizeof pty_name,
"%s", ptyname); }])
+ AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptyname = 0; sigset_t blocked;
sigemptyset (&blocked); sigaddset (&blocked, SIGCHLD); pthread_sigmask
(SIG_BLOCK, &blocked, 0); if (grantpt (fd) != -1 && unlockpt (fd) != -1)
ptyname = ptsname(fd); pthread_sigmask (SIG_UNBLOCK, &blocked, 0); if
(!ptyname) { emacs_close (fd); return -1; } snprintf (pty_name, sizeof
pty_name, "%s", ptyname); }])
dnl if HAVE_POSIX_OPENPT
if test "x$ac_cv_func_posix_openpt" = xyes; then
AC_DEFINE(PTY_OPEN, [fd = posix_openpt (O_RDWR | O_CLOEXEC |
O_NOCTTY)])
=== modified file 'src/ChangeLog'
--- a/src/ChangeLog 2013-07-11 17:28:58 +0000
+++ b/src/ChangeLog 2013-07-12 02:03:47 +0000
@@ -1,3 +1,28 @@
+2013-07-12 Paul Eggert <address@hidden>
+
+ Fix races with threads and file descriptors.
+ * callproc.c (Fcall_process_region):
+ * dired.c (open_directory):
+ * emacs.c (main, Fdaemon_initialized):
+ * image.c (x_find_image_file):
+ * inotify.c (Finotify_rm_watch):
+ * lread.c (Flocate_file_internal):
+ * process.c (Fnetwork_interface_list, Fnetwork_interface_info):
+ * term.c (term_mouse_moveto, init_tty):
+ * termcap.c (tgetent):
+ * unexaix.c, unexcoff.c (report_error, report_error_1, adjust_lnnoptrs)
+ * unexaix.c, unexcoff.c, unexcw.c, unexelf.c (unexec):
+ * unexhp9k800.c, unexmacosx.c (unexec):
+ * callproc.c (Fcall_process_region):
+ Use emacs_close, not close.
+ * sysdep.c (POSIX_CLOSE_RESTART, posix_close) [!POSIX_CLOSE_RESTART]:
+ New macro and function, which emulates the POSIX_CLOSE_RESTART macro
+ and posix_close function on current platforms (which all lack them).
+ (emacs_close): Use it. This should fix the races on GNU/Linux and
+ on AIX and on future platforms that support POSIX_CLOSE_RESTART,
+ and it should avoid closing random victim file descriptors on
+ other platforms.
+
2013-07-11 Paul Eggert <address@hidden>
* inotify.c (uninitialized): Remove. All uses replaced by -1.
=== modified file 'src/callproc.c'
--- a/src/callproc.c 2013-07-09 07:04:48 +0000
+++ b/src/callproc.c 2013-07-12 02:03:47 +0000
@@ -1052,7 +1052,7 @@
report_file_error ("Failed to open temporary file",
Fcons (build_string (tempfile), Qnil));
else
- close (fd);
+ emacs_close (fd);
}
#else
errno = 0;
=== modified file 'src/dired.c'
--- a/src/dired.c 2013-04-02 01:54:56 +0000
+++ b/src/dired.c 2013-07-12 02:03:47 +0000
@@ -95,7 +95,7 @@
d = fdopendir (fd);
opendir_errno = errno;
if (! d)
- close (fd);
+ emacs_close (fd);
}
#endif
=== modified file 'src/emacs.c'
--- a/src/emacs.c 2013-07-10 23:23:57 +0000
+++ b/src/emacs.c 2013-07-12 02:03:47 +0000
@@ -1010,7 +1010,7 @@
char buf[1];
/* Close unused writing end of the pipe. */
- close (daemon_pipe[1]);
+ emacs_close (daemon_pipe[1]);
/* Just wait for the child to close its end of the pipe. */
do
@@ -1030,7 +1030,7 @@
exit (1);
}
- close (daemon_pipe[0]);
+ emacs_close (daemon_pipe[0]);
exit (0);
}
if (f < 0)
@@ -1080,7 +1080,7 @@
if (dname_arg)
daemon_name = xstrdup (dname_arg);
/* Close unused reading end of the pipe. */
- close (daemon_pipe[0]);
+ emacs_close (daemon_pipe[0]);
setsid ();
#else /* DOS_NT */
@@ -2254,7 +2254,7 @@
err |= dup2 (nfd, 0) < 0;
err |= dup2 (nfd, 1) < 0;
err |= dup2 (nfd, 2) < 0;
- err |= close (nfd) != 0;
+ err |= emacs_close (nfd) != 0;
/* Closing the pipe will notify the parent that it can exit.
FIXME: In case some other process inherited the pipe, closing it here
@@ -2264,7 +2264,7 @@
call-process to make sure the pipe is never inherited by
subprocesses. */
err |= write (daemon_pipe[1], "\n", 1) < 0;
- err |= close (daemon_pipe[1]) != 0;
+ err |= emacs_close (daemon_pipe[1]) != 0;
/* Set it to an invalid value so we know we've already run this function. */
daemon_pipe[1] = -1;
=== modified file 'src/image.c'
--- a/src/image.c 2013-07-09 05:04:45 +0000
+++ b/src/image.c 2013-07-12 02:03:47 +0000
@@ -2260,7 +2260,7 @@
else
{
file_found = ENCODE_FILE (file_found);
- close (fd);
+ emacs_close (fd);
}
return file_found;
=== modified file 'src/inotify.c'
--- a/src/inotify.c 2013-07-11 17:28:58 +0000
+++ b/src/inotify.c 2013-07-12 02:03:47 +0000
@@ -387,7 +387,7 @@
/* Cleanup if no more files are watched. */
if (NILP (watch_list))
{
- close (inotifyfd);
+ emacs_close (inotifyfd);
delete_read_fd (inotifyfd);
inotifyfd = -1;
}
=== modified file 'src/lread.c'
--- a/src/lread.c 2013-07-10 23:23:57 +0000
+++ b/src/lread.c 2013-07-12 02:03:47 +0000
@@ -1413,7 +1413,7 @@
Lisp_Object file;
int fd = openp (path, filename, suffixes, &file, predicate);
if (NILP (predicate) && fd > 0)
- close (fd);
+ emacs_close (fd);
return file;
}
=== modified file 'src/process.c'
--- a/src/process.c 2013-07-09 07:04:48 +0000
+++ b/src/process.c 2013-07-12 02:03:47 +0000
@@ -3555,14 +3555,14 @@
ifconf.ifc_len = buf_size;
if (ioctl (s, SIOCGIFCONF, &ifconf))
{
- close (s);
+ emacs_close (s);
xfree (buf);
return Qnil;
}
}
while (ifconf.ifc_len == buf_size);
- close (s);
+ emacs_close (s);
res = Qnil;
ifreq = ifconf.ifc_req;
@@ -3819,7 +3819,7 @@
#endif
res = Fcons (elt, res);
- close (s);
+ emacs_close (s);
return any ? res : Qnil;
}
=== modified file 'src/sysdep.c'
--- a/src/sysdep.c 2013-07-11 02:17:47 +0000
+++ b/src/sysdep.c 2013-07-12 02:03:47 +0000
@@ -2201,23 +2201,59 @@
return fd < 0 ? 0 : fdopen (fd, mode);
}
+/* Approximate posix_close and POSIX_CLOSE_RESTART well enough for Emacs.
+ For the background behind this mess, please see Austin Group defect 529
+ <http://austingroupbugs.net/view.php?id=529>. */
+
+#ifndef POSIX_CLOSE_RESTART
+# define POSIX_CLOSE_RESTART 1
+static int
+posix_close (int fd, int flag)
+{
+ /* Only the POSIX_CLOSE_RESTART case is emulated. */
+ eassert (flag == POSIX_CLOSE_RESTART);
+
+ /* Things are tricky if close (fd) returns -1 with errno == EINTR
+ on a system that does not define POSIX_CLOSE_RESTART.
+
+ In this case, in some systems (e.g., GNU/Linux, AIX) FD is
+ closed, and retrying the close could inadvertently close a file
+ descriptor allocated by some other thread. In other systems
+ (e.g., HP/UX) FD is not closed. And in still other systems
+ (e.g., OS X, Solaris), maybe FD is closed, maybe not, and in a
+ multithreaded program there can be no way to tell.
+
+ So, in this case, pretend that the close succeeded. This works
+ well on systems like GNU/Linux that close FD. Although it may
+ leak a file descriptor on other systems, the leak is unlikely and
+ it's better to leak than to close a random victim. */
+ return close (fd) == 0 || errno == EINTR ? 0 : -1;
+}
+#endif
+
+/* Close FD, retrying if interrupted. If successful, return 0;
+ otherwise, return -1 and set errno to a non-EINTR value. Consider
+ an EINPROGRESS error to be successful, as that's merely a signal
+ arriving. FD is always closed when this function returns, even
+ when it returns -1.
+
+ Do not call this function if FD might already be closed, as that
+ might close an innocent victim opened by some other thread. */
+
int
emacs_close (int fd)
{
- bool did_retry = 0;
- int rtnval;
-
- while ((rtnval = close (fd)) == -1
- && (errno == EINTR))
- did_retry = 1;
-
- /* If close is interrupted SunOS 4.1 may or may not have closed the
- file descriptor. If it did the second close will fail with
- errno = EBADF. That means we have succeeded. */
- if (rtnval == -1 && did_retry && errno == EBADF)
- return 0;
-
- return rtnval;
+ while (1)
+ {
+ int r = posix_close (fd, POSIX_CLOSE_RESTART);
+ if (r == 0)
+ return r;
+ if (!POSIX_CLOSE_RESTART || errno != EINTR)
+ {
+ eassert (errno != EBADF);
+ return errno == EINPROGRESS ? 0 : r;
+ }
+ }
}
/* Maximum number of bytes to read or write in a single system call.
=== modified file 'src/term.c'
--- a/src/term.c 2013-07-11 02:17:47 +0000
+++ b/src/term.c 2013-07-12 02:03:47 +0000
@@ -2478,7 +2478,7 @@
name = (const char *) ttyname (0);
fd = emacs_open (name, O_WRONLY, 0);
SOME_FUNCTION (x, y, fd);
- close (fd);
+ emacs_close (fd);
last_mouse_x = x;
last_mouse_y = y; */
}
@@ -3012,7 +3012,7 @@
name);
if (!isatty (fd))
{
- close (fd);
+ emacs_close (fd);
maybe_fatal (must_succeed, terminal,
"Not a tty device: %s",
"Not a tty device: %s",
=== modified file 'src/termcap.c'
--- a/src/termcap.c 2013-07-11 02:17:47 +0000
+++ b/src/termcap.c 2013-07-12 02:03:47 +0000
@@ -455,7 +455,7 @@
/* Scan the file, reading it via buf, till find start of main entry. */
if (scan_file (term, fd, &buf) == 0)
{
- close (fd);
+ emacs_close (fd);
xfree (buf.beg);
if (malloc_size)
xfree (bp);
@@ -493,7 +493,7 @@
term = tgetst1 (tc_search_point, (char **) 0);
}
- close (fd);
+ emacs_close (fd);
xfree (buf.beg);
if (malloc_size)
=== modified file 'src/unexaix.c'
--- a/src/unexaix.c 2013-07-06 02:40:50 +0000
+++ b/src/unexaix.c 2013-07-12 02:03:47 +0000
@@ -97,7 +97,7 @@
if (fd)
{
int failed_errno = errno;
- close (fd);
+ emacs_close (fd);
errno = failed_errno;
}
report_file_error ("Cannot unexec", Fcons (build_string (file), Qnil));
@@ -111,7 +111,7 @@
report_error_1 (int fd, const char *msg, ...)
{
va_list ap;
- close (fd);
+ emacs_close (fd);
va_start (ap, msg);
verror (msg, ap);
va_end (ap);
@@ -148,13 +148,13 @@
|| adjust_lnnoptrs (new, a_out, new_name) < 0
|| unrelocate_symbols (new, a_out, a_name, new_name) < 0)
{
- close (new);
+ emacs_close (new);
return;
}
- close (new);
+ emacs_close (new);
if (a_out >= 0)
- close (a_out);
+ emacs_close (a_out);
mark_x (new_name);
}
@@ -534,7 +534,7 @@
}
}
}
- close (new);
+ emacs_close (new);
return 0;
}
=== modified file 'src/unexcoff.c'
--- a/src/unexcoff.c 2013-07-06 02:40:50 +0000
+++ b/src/unexcoff.c 2013-07-12 02:03:47 +0000
@@ -128,7 +128,7 @@
report_error (const char *file, int fd)
{
if (fd)
- close (fd);
+ emacs_close (fd);
report_file_error ("Cannot unexec", Fcons (build_string (file), Qnil));
}
@@ -139,7 +139,7 @@
static void
report_error_1 (int fd, const char *msg, int a1, int a2)
{
- close (fd);
+ emacs_close (fd);
error (msg, a1, a2);
}
@@ -511,7 +511,7 @@
}
}
#ifndef MSDOS
- close (new);
+ emacs_close (new);
#endif
return 0;
}
@@ -541,13 +541,13 @@
|| adjust_lnnoptrs (new, a_out, new_name) < 0
)
{
- close (new);
+ emacs_close (new);
return;
}
- close (new);
+ emacs_close (new);
if (a_out >= 0)
- close (a_out);
+ emacs_close (a_out);
mark_x (new_name);
}
=== modified file 'src/unexcw.c'
--- a/src/unexcw.c 2013-07-06 02:40:50 +0000
+++ b/src/unexcw.c 2013-07-12 02:03:47 +0000
@@ -316,13 +316,13 @@
ret2 = write (fd_out, buffer, ret);
assert (ret2 == ret);
}
- ret = close (fd_in);
+ ret = emacs_close (fd_in);
assert (ret == 0);
bss_sbrk_did_unexec = 1;
fixup_executable (fd_out);
bss_sbrk_did_unexec = 0;
- ret = close (fd_out);
+ ret = emacs_close (fd_out);
assert (ret == 0);
}
=== modified file 'src/unexelf.c'
--- a/src/unexelf.c 2013-07-06 02:40:50 +0000
+++ b/src/unexelf.c 2013-07-12 02:03:47 +0000
@@ -1312,13 +1312,13 @@
/* Close the files and make the new file executable. */
#if MAP_ANON == 0
- close (mmap_fd);
+ emacs_close (mmap_fd);
#endif
- if (close (old_file) != 0)
+ if (emacs_close (old_file) != 0)
fatal ("Can't close (%s): %s", old_name, strerror (errno));
- if (close (new_file) != 0)
+ if (emacs_close (new_file) != 0)
fatal ("Can't close (%s): %s", new_name, strerror (errno));
if (stat (new_name, &stat_buf) != 0)
=== modified file 'src/unexhp9k800.c'
--- a/src/unexhp9k800.c 2013-07-06 02:40:50 +0000
+++ b/src/unexhp9k800.c 2013-07-12 02:03:47 +0000
@@ -306,6 +306,6 @@
write_header (new, &hdr, &auxhdr);
/* Close the binary file */
- close (old);
- close (new);
+ emacs_close (old);
+ emacs_close (new);
}
=== modified file 'src/unexmacosx.c'
--- a/src/unexmacosx.c 2013-07-06 02:40:50 +0000
+++ b/src/unexmacosx.c 2013-07-12 02:03:47 +0000
@@ -1332,7 +1332,7 @@
outfd = emacs_open (outfile, O_WRONLY | O_TRUNC | O_CREAT, 0755);
if (outfd < 0)
{
- close (infd);
+ emacs_close (infd);
unexec_error ("cannot open output file `%s'", outfile);
}
@@ -1346,7 +1346,7 @@
dump_it ();
- close (outfd);
+ emacs_close (outfd);
}
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [Emacs-diffs] trunk r113384: Fix races with threads and file descriptors.,
Paul Eggert <=