Change 33502 by [EMAIL PROTECTED] on 2008/03/12 22:03:57
Integrate:
[ 33501]
Integrate:
[ 33491]
Correct logic error in PerlIOStdio_close() - 0 is an acceptable value
from dup(), so it can't also be the "don't do anything later" value.
[ 33492]
We need mutex protection in PerlIOStdio_close() for the duration of
holding our true love file handle open, to stop anything else
temporarily using it for a quick dup() fling, and then closing the
file handle underneath us.
I suspect that the lack of this protection was the cause of the threads
free.t and blocks.t failures on OS X on 5.8.x, where usefaststdio is
the default, and PerlIO is unable to "invalidate" the FILE *.
[ 33498]
Change 33492 did not spread the protection wide enough. There were
still two more races to be lost.
1: The close() could still happen after the (premature) mutex release
allowed another thread to dup() to that file descriptor.
2: The initial dup() could happen whilst another thread was in the
mutex protected region, and had temporarily closed the file
descriptor.
Race conditions remain with any other thread that actually does I/O
during the execution of the mutex protected region (as noted in a
comment), and dup() failure is not handled gracefully (also noted).
Affected files ...
... //depot/maint-5.8/perl/perlio.c#118 integrate
Differences ...
==== //depot/maint-5.8/perl/perlio.c#118 (text) ====
Index: perl/perlio.c
--- perl/perlio.c#117~32544~ 2007-11-28 15:06:16.000000000 -0800
+++ perl/perlio.c 2008-03-12 15:03:57.000000000 -0700
@@ -3114,7 +3114,7 @@
int invalidate = 0;
IV result = 0;
int saveerr = 0;
- int dupfd = 0;
+ int dupfd = -1;
#ifdef SOCKS5_VERSION_NAME
/* Socks lib overrides close() but stdio isn't linked to
that library (though we are) - so we must call close()
@@ -3140,8 +3140,37 @@
result = PerlIO_flush(f);
saveerr = errno;
invalidate = PerlIOStdio_invalidate_fileno(aTHX_ stdio);
- if (!invalidate)
+ if (!invalidate) {
+#ifdef USE_ITHREADS
+ MUTEX_LOCK(&PL_perlio_mutex);
+ /* Right. We need a mutex here because for a brief while we will
+ have the situation that fd is actually closed. Hence if a
+ second thread were to get into this block, its dup() would
+ likely return our fd as its dupfd. (after all, it is closed).
+ Then if we get to the dup2() first, we blat the fd back
+ (messing up its temporary as a side effect) only for it to
+ then close its dupfd (== our fd) in its close(dupfd) */
+
+ /* There is, of course, a race condition, that any other thread
+ trying to input/output/whatever on this fd will be stuffed
+ for the duraction of this little manoeuver. Perhaps we should
+ hold an IO mutex for the duration of every IO operation if
+ we know that invalidate doesn't work on this platform, but
+ that would suck, and could kill performance.
+
+ Except that correctness trumps speed.
+ Advice from klortho #11912. */
+#endif
dupfd = PerlLIO_dup(fd);
+#ifdef USE_ITHREADS
+ if (dupfd < 0) {
+ MUTEX_UNLOCK(&PL_perlio_mutex);
+ /* Oh cXap. This isn't going to go well. Not sure if we can
+ recover from here, or if closing this particular FILE *
+ is a good idea now. */
+ }
+#endif
+ }
}
result = PerlSIO_fclose(stdio);
/* We treat error from stdio as success if we invalidated
@@ -3155,9 +3184,12 @@
/* in SOCKS' case, let close() determine return value */
result = close(fd);
#endif
- if (dupfd) {
+ if (dupfd >= 0) {
PerlLIO_dup2(dupfd,fd);
PerlLIO_close(dupfd);
+#ifdef USE_ITHREADS
+ MUTEX_UNLOCK(&PL_perlio_mutex);
+#endif
}
return result;
}
End of Patch.
