Change 16333 by gsar@onru on 2002/05/02 07:08:42
fix yet more race conditions related to fdopen() and dup2():
fclose() is not thread-safe when two FILE* structures point
to the same underlying fd, as it happens in perl's socket
handles; need to invalidate the fileno slot of one of the
FILE* structures, but unfortunately stdio has no interface
to do this; we can do little else but fiddle with the
FILE* structure directly (yuck! hope this could be done better
under native PerlIO in 5.8)
Affected files ...
.... //depot/maint-5.6/perl/doio.c#20 edit
.... //depot/maint-5.6/perl/embed.h#37 edit
.... //depot/maint-5.6/perl/embed.pl#53 edit
.... //depot/maint-5.6/perl/objXSUB.h#30 edit
.... //depot/maint-5.6/perl/perlapi.c#33 edit
.... //depot/maint-5.6/perl/proto.h#42 edit
Differences ...
==== //depot/maint-5.6/perl/doio.c#20 (text) ====
Index: perl/doio.c
--- perl/doio.c.~1~ Thu May 2 01:15:05 2002
+++ perl/doio.c Thu May 2 01:15:05 2002
@@ -51,6 +51,61 @@
#include <signal.h>
#endif
+#if defined(USE_ITHREADS)
+STATIC void
+S_invalidate_fileno(pTHX_ PerlIO *f)
+{
+ int fd = PerlIO_fileno(f);
+ PerlIO_flush(f);
+# if defined(USE_SFIO)
+# error "dont know how to set FILE.fileno under sfio"
+# endif
+ /* XXX this could use PerlIO_canset_fileno() and
+ * PerlIO_set_fileno() support from Configure */
+# if defined(__GLIBC__)
+ ((FILE*)f)->_fileno = -1;
+# elif defined(__sun__)
+ /* _file is just a char :-( */
+ ((FILE*)f)->_file = PerlLIO_dup(fd);
+# elif defined(__hpux)
+ ((FILE*)f)->__fileH = 0xff;
+ ((FILE*)f)->__fileL = 0xff;
+# elif defined(__FreeBSD__)
+ ((FILE*)f)->_file = -1;
+# elif defined(WIN32)
+# if defined(__BORLANDC__)
+ ((FILE*)f)->fd = PerlLIO_dup(fd);
+# else
+ ((FILE*)f)->_file = -1;
+# endif
+# else
+# error "dont know how to set FILE.fileno on your platform"
+# endif
+}
+#endif
+
+STATIC int
+S_io_sock_close(pTHX_ IO *io)
+{
+ int result;
+
+#if defined(USE_ITHREADS)
+ /* Avoid race condition: without this, the second fclose() will
+ * attempt to close() the same fd, and that fd could have been
+ * allocated by another thread between the two fclose() calls.
+ * It is potentially better to keep the two fds separate by making
+ * one a dup() of the other, but doing so muddies the perl-level
+ * semantics more than this hack. What should fileno(SOCK) return
+ * in that case? How about fcntl(SOCK,...)? Etc. */
+ if (PerlIO_fileno(IoIFP(io)) == PerlIO_fileno(IoOFP(io)))
+ invalidate_fileno(IoIFP(io));
+#endif
+ result = PerlIO_close(IoOFP(io));
+ PerlIO_close(IoIFP(io)); /* clear stdio, fd already closed */
+
+ return result;
+}
+
bool
Perl_do_open(pTHX_ GV *gv, register char *name, I32 len, int as_raw,
int rawmode, int rawperm, PerlIO *supplied_fp)
@@ -99,10 +154,8 @@
else if (IoTYPE(io) == IoTYPE_PIPE)
result = PerlProc_pclose(IoIFP(io));
else if (IoIFP(io) != IoOFP(io)) {
- if (IoOFP(io)) {
- result = PerlIO_close(IoOFP(io));
- PerlIO_close(IoIFP(io)); /* clear stdio, fd already closed */
- }
+ if (IoOFP(io))
+ result = io_sock_close(io);
else
result = PerlIO_close(IoIFP(io));
}
@@ -458,6 +511,10 @@
if (saveofp) {
PerlIO_flush(saveofp); /* emulate PerlIO_close() */
if (saveofp != saveifp) { /* was a socket? */
+#if defined(USE_ITHREADS)
+ if (fd == PerlIO_fileno(saveofp))
+ invalidate_fileno(saveofp);
+#endif
PerlIO_close(saveofp);
}
}
@@ -496,11 +553,21 @@
if (was_fdopen) {
/* need to close fp without closing underlying fd */
+#if defined(USE_THREADS)
+ /* we do do this only in the non-ithreads case because of
+ * the platform-specific nature of invalidate_fileno() */
+ invalidate_fileno(fp);
+ PerlIO_close(fp);
+#else
int ofd = PerlIO_fileno(fp);
int dupfd = PerlLIO_dup(ofd);
PerlIO_close(fp);
+ /* there is a race condition here that makes this code
+ * thread-unsafe. ofd could have been allocated by
+ * another thread at this point. */
PerlLIO_dup2(dupfd,ofd);
PerlLIO_close(dupfd);
+#endif
}
else
PerlIO_close(fp);
@@ -859,10 +926,8 @@
else if (IoTYPE(io) == IoTYPE_STD)
retval = TRUE;
else {
- if (IoOFP(io) && IoOFP(io) != IoIFP(io)) { /* a socket */
- retval = (PerlIO_close(IoOFP(io)) != EOF);
- PerlIO_close(IoIFP(io)); /* clear stdio, fd already closed */
- }
+ if (IoOFP(io) && IoOFP(io) != IoIFP(io)) /* a socket */
+ retval = (io_sock_close(io) != EOF);
else
retval = (PerlIO_close(IoIFP(io)) != EOF);
}
@@ -1400,7 +1465,7 @@
while (*t && isSPACE(*t))
++t;
- if (!*t && (dup2(1,2) != -1)) {
+ if (!*t && (PerlLIO_dup2(1,2) != -1)) {
s[-2] = '\0';
break;
}
==== //depot/maint-5.6/perl/embed.h#37 (text+w) ====
Index: perl/embed.h
--- perl/embed.h.~1~ Thu May 2 01:15:05 2002
+++ perl/embed.h Thu May 2 01:15:05 2002
@@ -863,6 +863,12 @@
#define avhv_index_sv S_avhv_index_sv
#define avhv_index S_avhv_index
#endif
+#if defined(PERL_IN_DOIO_C) || defined(PERL_DECL_PROT)
+#define io_sock_close S_io_sock_close
+#if defined(USE_ITHREADS)
+#define invalidate_fileno S_invalidate_fileno
+#endif
+#endif
#if defined(PERL_IN_DOOP_C) || defined(PERL_DECL_PROT)
#define do_trans_simple S_do_trans_simple
#define do_trans_count S_do_trans_count
@@ -2321,6 +2327,12 @@
#define avhv_index_sv(a) S_avhv_index_sv(aTHX_ a)
#define avhv_index(a,b,c) S_avhv_index(aTHX_ a,b,c)
#endif
+#if defined(PERL_IN_DOIO_C) || defined(PERL_DECL_PROT)
+#define io_sock_close(a) S_io_sock_close(aTHX_ a)
+#if defined(USE_ITHREADS)
+#define invalidate_fileno(a) S_invalidate_fileno(aTHX_ a)
+#endif
+#endif
#if defined(PERL_IN_DOOP_C) || defined(PERL_DECL_PROT)
#define do_trans_simple(a) S_do_trans_simple(aTHX_ a)
#define do_trans_count(a) S_do_trans_count(aTHX_ a)
@@ -4544,6 +4556,14 @@
#define S_avhv_index CPerlObj::S_avhv_index
#define avhv_index S_avhv_index
#endif
+#if defined(PERL_IN_DOIO_C) || defined(PERL_DECL_PROT)
+#define S_io_sock_close CPerlObj::S_io_sock_close
+#define io_sock_close S_io_sock_close
+#if defined(USE_ITHREADS)
+#define S_invalidate_fileno CPerlObj::S_invalidate_fileno
+#define invalidate_fileno S_invalidate_fileno
+#endif
+#endif
#if defined(PERL_IN_DOOP_C) || defined(PERL_DECL_PROT)
#define S_do_trans_simple CPerlObj::S_do_trans_simple
#define do_trans_simple S_do_trans_simple
==== //depot/maint-5.6/perl/embed.pl#53 (xtext) ====
Index: perl/embed.pl
--- perl/embed.pl.~1~ Thu May 2 01:15:05 2002
+++ perl/embed.pl Thu May 2 01:15:05 2002
@@ -2232,6 +2232,13 @@
s |I32 |avhv_index |AV* av|SV* sv|U32 hash
#endif
+#if defined(PERL_IN_DOIO_C) || defined(PERL_DECL_PROT)
+s |int |io_sock_close |IO *io
+#if defined(USE_ITHREADS)
+s |void |invalidate_fileno|PerlIO *f
+#endif
+#endif
+
#if defined(PERL_IN_DOOP_C) || defined(PERL_DECL_PROT)
s |I32 |do_trans_simple |SV *sv
s |I32 |do_trans_count |SV *sv
==== //depot/maint-5.6/perl/objXSUB.h#30 (text+w) ====
Index: perl/objXSUB.h
--- perl/objXSUB.h.~1~ Thu May 2 01:15:05 2002
+++ perl/objXSUB.h Thu May 2 01:15:05 2002
@@ -2268,6 +2268,10 @@
#endif
#if defined(PERL_IN_AV_C) || defined(PERL_DECL_PROT)
#endif
+#if defined(PERL_IN_DOIO_C) || defined(PERL_DECL_PROT)
+#if defined(USE_ITHREADS)
+#endif
+#endif
#if defined(PERL_IN_DOOP_C) || defined(PERL_DECL_PROT)
#endif
#if defined(PERL_IN_GV_C) || defined(PERL_DECL_PROT)
==== //depot/maint-5.6/perl/perlapi.c#33 (text+w) ====
Index: perl/perlapi.c
--- perl/perlapi.c.~1~ Thu May 2 01:15:05 2002
+++ perl/perlapi.c Thu May 2 01:15:05 2002
@@ -4082,6 +4082,10 @@
#endif
#if defined(PERL_IN_AV_C) || defined(PERL_DECL_PROT)
#endif
+#if defined(PERL_IN_DOIO_C) || defined(PERL_DECL_PROT)
+#if defined(USE_ITHREADS)
+#endif
+#endif
#if defined(PERL_IN_DOOP_C) || defined(PERL_DECL_PROT)
#endif
#if defined(PERL_IN_GV_C) || defined(PERL_DECL_PROT)
==== //depot/maint-5.6/perl/proto.h#42 (text+w) ====
Index: perl/proto.h
--- perl/proto.h.~1~ Thu May 2 01:15:05 2002
+++ perl/proto.h Thu May 2 01:15:05 2002
@@ -972,6 +972,13 @@
STATIC I32 S_avhv_index(pTHX_ AV* av, SV* sv, U32 hash);
#endif
+#if defined(PERL_IN_DOIO_C) || defined(PERL_DECL_PROT)
+STATIC int S_io_sock_close(pTHX_ IO *io);
+#if defined(USE_ITHREADS)
+STATIC void S_invalidate_fileno(pTHX_ PerlIO *f);
+#endif
+#endif
+
#if defined(PERL_IN_DOOP_C) || defined(PERL_DECL_PROT)
STATIC I32 S_do_trans_simple(pTHX_ SV *sv);
STATIC I32 S_do_trans_count(pTHX_ SV *sv);
End of Patch.
