Skip to content

Commit af23c88

Browse files
committed
_sim_debug_flush(): Use fsync().
Fix race condition in _debug_fwrite_all() where the FILE *f output file can be permanently NULL, causing fwrite() to permanently return zero and resulting in an infinite loop on mulithreaded SIMH. This pathology primarily occurs when main thread calls flush_svc() and one of the Ethernet threads (reader, writer) emits debugging output. Linux and macOS platforms invoke fsync() to synchronize file data to disk. Windows has a fsync() wrapper calling _commit() providing the same semantics. Issue traced to _sim_debug_flush() calling sim_set_deboff(0, NULL) to simulate fsync().
1 parent c20b391 commit af23c88

1 file changed

Lines changed: 45 additions & 30 deletions

File tree

scp.c

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,19 @@
253253
#include ".git-commit-id.h"
254254
#endif
255255

256+
#if !defined(_WIN32) && !defined(_WIN64)
257+
/* fsync() */
258+
#include <unistd.h>
259+
#elif defined(_WIN32) || defined(_WIN64)
260+
#include <io.h>
261+
262+
/* fsync() wrapper for Windows. */
263+
static inline int fsync(int fd)
264+
{
265+
return _commit(fd);
266+
}
267+
#endif
268+
256269
#ifndef MAX
257270
#define MAX(a,b) (((a) >= (b)) ? (a) : (b))
258271
#endif
@@ -13611,14 +13624,36 @@ static void _debug_fwrite_all (const char *buf, size_t len, FILE *f)
1361113624
{
1361213625
size_t len_written;
1361313626

13614-
while (len > 0) {
13615-
errno = 0;
13616-
len_written = fwrite (buf, 1, len, f);
13617-
len -= len_written;
13618-
buf += len_written;
13619-
if (errno == EAGAIN) /* Non blocking file descriptor buffer full? */
13620-
sim_os_ms_sleep(10);/* wait a bit to retry */
13621-
}
13627+
/* NOTE: There be a dragon here. This is UNSAFE in a multithreaded SIMH. This
13628+
* race could still exist if sim_set_deboff() is called prematurely before all
13629+
* threads are quiescent (needs confirmation.)
13630+
*
13631+
* _sim_debug_flush() calls sim_set_deboff(0, NULL), which NULLs sim_deb (aka f).
13632+
* This causes a race on sim_deb between threads -- a thread can read a NULL sim_deb
13633+
* and call _debug_fwrite_all() with f == NULL.
13634+
*
13635+
* f == NULL: fwrite() returns 0 on Linux and Windows. len never gets decremented
13636+
* and an infinite loop ensues (cue "Forever NULL" sung to the tune of Alphaville's
13637+
* "Forever Young".)
13638+
*/
13639+
if (f != NULL) {
13640+
while (len > 0) {
13641+
errno = 0;
13642+
len_written = fwrite (buf, 1, len, f);
13643+
len -= len_written;
13644+
buf += len_written;
13645+
if (errno == EAGAIN) /* Non blocking file descriptor buffer full? */
13646+
sim_os_ms_sleep(10);/* wait a bit to retry */
13647+
} else if (len_written == 0 || errno != EAGAIN) {
13648+
/* Deal with dragon (above). */
13649+
sim_perror("Debug output truncated/not written due to error.\n");
13650+
break;
13651+
}
13652+
} else {
13653+
/* Deal with the dragon, although, realisitically, this message should
13654+
* be rarely emitted. */
13655+
sim_perror("Debug output truncated/not written, FILE is NULL.\n");
13656+
}
1362213657
}
1362313658

1362413659
static void _debug_fwrite (const char *buf, size_t len)
@@ -13755,33 +13790,13 @@ _sim_debug_write_flush (buf, len, FALSE);
1375513790

1375613791
static t_stat _sim_debug_flush (void)
1375713792
{
13758-
int32 saved_quiet = sim_quiet;
13759-
int32 saved_sim_switches = sim_switches;
13760-
int32 saved_deb_switches = sim_deb_switches;
13761-
struct timespec saved_deb_basetime = sim_deb_basetime;
13762-
char saved_debug_filename[CBUFSIZE];
13763-
1376413793
if (sim_deb == NULL) /* no debug? */
1376513794
return SCPE_OK;
1376613795

1376713796
_sim_debug_write_flush ("", 0, TRUE);
13797+
fflush (sim_deb);
13798+
fsync(fileno(sim_deb));
1376813799

13769-
if (sim_deb == sim_log) { /* debug is log */
13770-
fflush (sim_deb); /* fflush is the best we can do */
13771-
return SCPE_OK;
13772-
}
13773-
13774-
if (!(saved_deb_switches & SWMASK ('B'))) {
13775-
strcpy (saved_debug_filename, sim_logfile_name (sim_deb, sim_deb_ref));
13776-
13777-
sim_quiet = 1;
13778-
sim_set_deboff (0, NULL);
13779-
sim_switches = saved_deb_switches;
13780-
sim_set_debon (0, saved_debug_filename);
13781-
sim_deb_basetime = saved_deb_basetime;
13782-
sim_switches = saved_sim_switches;
13783-
sim_quiet = saved_quiet;
13784-
}
1378513800
return SCPE_OK;
1378613801
}
1378713802

0 commit comments

Comments
 (0)