Skip to content

Commit 5b2ff71

Browse files
claudethewilsonator
authored andcommitted
Fix issue 11006 - EINTR signal causes spurious failure in std.stdio writes
A signal whose handler does not carry SA_RESTART can interrupt fwrite(3) mid-write, causing it to return a short count with errno == EINTR and the stream's error indicator set. Three write paths in std.stdio treated this as fatal: LockingTextWriter.put (byte-array fast path and per-character paths), File.rawWrite, and BinaryWriterImpl.rawWrite. Programs would crash on the next writeln after a signal landed mid-flush. Wrap the underlying fwrite/fputc/fputwc calls in std.internal.retry's retryShortIO and retryOnEINTR so EINTR is transparently retried until the buffer is fully written or a non-transient error occurs. #11006
1 parent b56cef1 commit 5b2ff71

1 file changed

Lines changed: 83 additions & 13 deletions

File tree

std/stdio.d

Lines changed: 83 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,6 +1093,7 @@ Throws: `ErrnoException` if the file is not opened or if the call to `fwrite` fa
10931093
{
10941094
import std.conv : text;
10951095
import std.exception : errnoEnforce;
1096+
import std.internal.retry : retryShortIO;
10961097

10971098
version (Windows)
10981099
{
@@ -1117,10 +1118,13 @@ Throws: `ErrnoException` if the file is not opened or if the call to `fwrite` fa
11171118
}
11181119
}
11191120

1120-
auto result = trustedFwrite(_p.handle, buffer);
1121-
if (result == result.max) result = 0;
1122-
errnoEnforce(result == buffer.length,
1123-
text("Wrote ", result, " instead of ", buffer.length,
1121+
FILE* h = _p.handle;
1122+
immutable n = (() @trusted => retryShortIO(
1123+
(size_t off) => trustedFwrite(h, buffer[off .. $]),
1124+
buffer.length,
1125+
() { .clearerr(h); }))();
1126+
errnoEnforce(n == buffer.length,
1127+
text("Wrote ", n, " instead of ", buffer.length,
11241128
" objects of type ", T.stringof, " to file `",
11251129
_name, "'"));
11261130
}
@@ -3106,14 +3110,18 @@ is empty, throws an `Exception`. In case of an I/O error throws
31063110
private void putcChecked(_iobuf* h, int c) @trusted
31073111
{
31083112
import std.exception : errnoEnforce;
3109-
if (trustedFPUTC(c, h) == EOF) errnoEnforce(0);
3113+
import std.internal.retry : retryOnEINTR;
3114+
if (retryOnEINTR(() => trustedFPUTC(c, h), EOF) == EOF)
3115+
errnoEnforce(0);
31103116
}
31113117

31123118
private void putwcChecked(_iobuf* h, wchar_t c) @trusted
31133119
{
31143120
import std.exception : errnoEnforce;
3121+
import std.internal.retry : retryOnEINTR;
31153122
// fputwc returns WEOF (= -1) on error
3116-
if (trustedFPUTWC(c, h) == -1) errnoEnforce(0);
3123+
if (retryOnEINTR(() => trustedFPUTWC(c, h)) == -1)
3124+
errnoEnforce(0);
31173125
}
31183126

31193127
/// Range primitive implementations.
@@ -3133,8 +3141,13 @@ is empty, throws an `Exception`. In case of an I/O error throws
31333141
{
31343142
//file.write(writeme); causes infinite recursion!!!
31353143
//file.rawWrite(writeme);
3136-
auto result = trustedFwrite(file_._p.handle, writeme);
3137-
if (result != writeme.length) errnoEnforce(0);
3144+
import std.internal.retry : retryShortIO;
3145+
FILE* h = file_._p.handle;
3146+
immutable n = (() @trusted => retryShortIO(
3147+
(size_t off) => trustedFwrite(h, writeme[off .. $]),
3148+
writeme.length,
3149+
() { .clearerr(h); }))();
3150+
if (n != writeme.length) errnoEnforce(0);
31383151
return;
31393152
}
31403153
}
@@ -3354,11 +3367,15 @@ is empty, throws an `Exception`. In case of an I/O error throws
33543367
{
33553368
import std.conv : text;
33563369
import std.exception : errnoEnforce;
3357-
3358-
auto result = trustedFwrite(file_._p.handle, buffer);
3359-
if (result == result.max) result = 0;
3360-
errnoEnforce(result == buffer.length,
3361-
text("Wrote ", result, " instead of ", buffer.length,
3370+
import std.internal.retry : retryShortIO;
3371+
3372+
FILE* h = file_._p.handle;
3373+
immutable n = (() @trusted => retryShortIO(
3374+
(size_t off) => trustedFwrite(h, buffer[off .. $]),
3375+
buffer.length,
3376+
() { .clearerr(h); }))();
3377+
errnoEnforce(n == buffer.length,
3378+
text("Wrote ", n, " instead of ", buffer.length,
33623379
" objects of type ", T.stringof, " to file `",
33633380
name, "'"));
33643381
}
@@ -3858,6 +3875,59 @@ version (linux)
38583875
assertThrown!ErrnoException(w.put('x'));
38593876
}
38603877

3878+
// https://github.com/dlang/phobos/issues/11006
3879+
// LockingTextWriter.put survives an EINTR from a signal without SA_RESTART.
3880+
version (linux)
3881+
@system unittest
3882+
{
3883+
import core.atomic : atomicLoad, atomicStore;
3884+
import core.sys.posix.pthread : pthread_kill;
3885+
import core.sys.posix.signal : SA_RESTART, SIGUSR1, sigaction, sigaction_t,
3886+
sigemptyset;
3887+
import core.sys.posix.unistd : read;
3888+
import core.thread : Thread;
3889+
import core.time : msecs;
3890+
import std.process : pipe;
3891+
3892+
// Install a SIGUSR1 handler without SA_RESTART so that fwrite may return
3893+
// short with errno == EINTR when the signal arrives mid-write.
3894+
extern (C) static nothrow void noop(int) {}
3895+
sigaction_t sa;
3896+
sa.sa_handler = &noop;
3897+
sigemptyset(&sa.sa_mask);
3898+
sa.sa_flags = 0; // no SA_RESTART
3899+
sigaction(SIGUSR1, &sa, null);
3900+
3901+
auto p = pipe();
3902+
// Write 512 KiB — much larger than the default 64 KiB pipe buffer —
3903+
// so the writer thread reliably blocks inside write(2).
3904+
immutable msg = new ubyte[512 * 1024];
3905+
3906+
shared bool done = false;
3907+
shared bool threw = false;
3908+
auto t = new Thread(() {
3909+
try
3910+
p.writeEnd.lockingTextWriter.put(cast(const(ubyte)[]) msg);
3911+
catch (Exception)
3912+
atomicStore(threw, true);
3913+
atomicStore(done, true);
3914+
});
3915+
t.start();
3916+
3917+
// Give the writer time to block inside write(2) before signalling.
3918+
Thread.sleep(50.msecs);
3919+
pthread_kill(t.id, SIGUSR1);
3920+
3921+
// Drain the pipe so the blocked write can make progress and complete.
3922+
ubyte[4096] buf;
3923+
while (!atomicLoad(done))
3924+
read(p.readEnd.fileno, buf.ptr, buf.length);
3925+
t.join();
3926+
3927+
assert(!atomicLoad(threw),
3928+
"LockingTextWriter.put threw on EINTR — retryShortIO not working");
3929+
}
3930+
38613931
@safe unittest // https://issues.dlang.org/show_bug.cgi?id=21592
38623932
{
38633933
import std.exception : collectException;

0 commit comments

Comments
 (0)