Re: io_write()/fwrite() and EINTR on Solaris
From:
Jos Backus <jos@...>
Date:
2002-11-22 17:34:23 UTC
List:
ruby-core #582
See below for a status update on this problem.
On Fri, Nov 08, 2002 at 04:08:44PM +0900, Jos Backus wrote:
> On Fri, Nov 08, 2002 at 02:10:52PM +0900, nobu.nokada@softhome.net wrote:
> OK, to test if the fputc() version works reliably I will modify the
> interpreter at work to use fputc() instead of fwrite().
>
> If it works satisfactorily I would prefer that we use the fputc() version on
> Solaris instead of the signal-blocking approach, even though it is less
> efficient - at least it does not introduce any change in semantics. I should
> know in a few days if the fputc() approach is safe (my colleague will complain
> if it isn't :) and will report back.
My tests here indicate that using fputc() on Solaris is not safe either.
I am currently working with a Sun TSE to resolve this issue. Meanwhile I am
using the following patch. Sadly I have not had time to do any of the
configure changes, but hopefully they won't be necessary if Sun issues a
patch. I'll keep you posted.
Note that the patch also changes the signature of rb_io_fread() to use size_t
instead of long for consistency's sake and because it's the proper type to
use. You may want to consider committing that change separately.
Index: io.c
===================================================================
RCS file: /src/ruby/io.c,v
retrieving revision 1.167
diff -u -r1.167 io.c
--- io.c 29 Oct 2002 21:35:28 -0000 1.167
+++ io.c 22 Nov 2002 17:24:34 -0000
@@ -24,6 +24,11 @@
#include <ctype.h>
#include <errno.h>
+#define BROKEN_FWRITE 1
+#ifdef BROKEN_FWRITE
+# include <signal.h>
+#endif
+
#if defined(MSDOS) || defined(__BOW__) || defined(__CYGWIN__) || defined(NT) || defined(__human68k__) || defined(__EMX__) || defined(__BEOS__)
# define NO_SAFE_RENAME
#endif
@@ -351,14 +356,92 @@
}
/* writing functions */
+#ifdef BROKEN_FWRITE
+# ifdef HAVE_SIGPROCMASK
+static sigset_t fwrite_mask;
+/* XXX How is this supposed to be used? --jos */
+# define Init_fwrite() (sigemptyset(&fwrite_mask), sigaddset(&fwrite_mask, SIGVTALRM))
+# else
+# define Init_fwrite() (void)0
+# endif
+
+static size_t
+safe_fwrite(ptr, len, f)
+ const void *ptr;
+ size_t len;
+ FILE *f;
+{
+ size_t n;
+# ifdef HAVE_SIGPROCMASK
+ sigset_t set;
+ static int init_fwrite_mask = 1;
+ if (init_fwrite_mask) {
+ sigemptyset(&fwrite_mask);
+ sigaddset(&fwrite_mask, SIGVTALRM);
+ init_fwrite_mask = 0;
+ }
+ sigprocmask(SIG_BLOCK, &fwrite_mask, &set);
+# else
+ int mask = siggetmask();
+ sigblock(sigmask(SIGVTALRM));
+# endif
+
+ n = fwrite(ptr, 1, len, f);
+
+# ifdef HAVE_SIGPROCMASK
+ sigprocmask(SIG_UNBLOCK, &set, 0);
+# else
+ sigsetmask(mask);
+# endif
+ return n;
+}
+
+#else
+# define safe_fwrite(p, n, f) fwrite((p), 1, (n), (f))
+#endif
+
+size_t
+rb_io_fwrite(ptr, len, f)
+ const char *ptr;
+ size_t len;
+ FILE *f;
+{
+ size_t n, r;
+
+ if ((n = len) <= 0) return n;
+#if defined(__human68k__)
+ do {
+ if (fputc(*ptr, f) == EOF) {
+ if (!ferror(f)) break;
+ if (rb_io_wait_writable(fileno(f))) {
+ clearerr(f);
+ continue;
+ }
+ return -1L;
+ }
+ ++ptr;
+ } while (--n > 0);
+#else
+ while (ptr += (r = safe_fwrite(ptr, n, f)), (n -= r) > 0) {
+ if (ferror(f)) {
+ if (rb_io_wait_writable(fileno(f))) {
+ clearerr(f);
+ continue;
+ }
+ return -1L;
+ }
+ }
+#endif
+ return len - n;
+}
+
static VALUE
io_write(io, str)
VALUE io, str;
{
OpenFile *fptr;
FILE *f;
- long n, r;
- register char *ptr;
+ long n;
rb_secure(4);
if (TYPE(str) != T_STRING)
@@ -374,27 +457,8 @@
rb_io_check_writable(fptr);
f = GetWriteFile(fptr);
- ptr = RSTRING(str)->ptr;
- n = RSTRING(str)->len;
-#ifdef __human68k__
- do {
- if (fputc(*ptr++, f) == EOF) {
- if (ferror(f)) rb_sys_fail(fptr->path);
- break;
- }
- } while (--n > 0);
-#else
- while (ptr += (r = fwrite(ptr, 1, n, f)), (n -= r) > 0) {
- if (ferror(f)) {
- if (rb_io_wait_writable(fileno(f))) {
- clearerr(f);
- continue;
- }
- rb_sys_fail(fptr->path);
- }
- }
-#endif
- n = ptr - RSTRING(str)->ptr;
+ n = rb_io_fwrite(RSTRING(str)->ptr, RSTRING(str)->len, f);
+ if (n == -1L) rb_sys_fail(fptr->path);
if (fptr->mode & FMODE_SYNC) {
io_fflush(f, fptr);
}
@@ -638,10 +702,10 @@
}
/* reading functions */
-long
+size_t
rb_io_fread(ptr, len, f)
char *ptr;
- long len;
+ size_t len;
FILE *f;
{
long n = len;
Index: marshal.c
===================================================================
RCS file: /src/ruby/marshal.c,v
retrieving revision 1.73
diff -u -r1.73 marshal.c
--- marshal.c 17 Oct 2002 10:20:52 -0000 1.73
+++ marshal.c 22 Nov 2002 17:24:35 -0000
@@ -102,7 +102,8 @@
struct dump_arg *arg;
{
if (arg->fp) {
- fwrite(s, 1, n, arg->fp);
+ if (rb_io_fwrite(s, n, arg->fp) < 0)
+ rb_sys_fail(0);
}
else {
VALUE buf = arg->str;
Index: rubyio.h
===================================================================
RCS file: /src/ruby/rubyio.h,v
retrieving revision 1.20
diff -u -r1.20 rubyio.h
--- rubyio.h 2 Oct 2002 14:59:25 -0000 1.20
+++ rubyio.h 22 Nov 2002 17:24:35 -0000
@@ -58,7 +58,8 @@
FILE *rb_fopen _((const char*, const char*));
FILE *rb_fdopen _((int, const char*));
int rb_getc _((FILE*));
-long rb_io_fread _((char *, long, FILE *));
+size_t rb_io_fread _((char *, size_t, FILE *));
+size_t rb_io_fwrite _((const char *, size_t, FILE *));
int rb_io_mode_flags _((const char*));
void rb_io_check_writable _((OpenFile*));
void rb_io_check_readable _((OpenFile*));
--
Jos Backus _/ _/_/_/ Sunnyvale, CA
_/ _/ _/
_/ _/_/_/
_/ _/ _/ _/
jos at catnook.com _/_/ _/_/_/ require 'std/disclaimer'