Skip to content

Commit 2e6a571

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 b036821 commit 2e6a571

1 file changed

Lines changed: 27 additions & 22 deletions

File tree

scp.c

Lines changed: 27 additions & 22 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,6 +13624,18 @@ static void _debug_fwrite_all (const char *buf, size_t len, FILE *f)
1361113624
{
1361213625
size_t len_written;
1361313626

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+
*/
1361413639
while (len > 0) {
1361513640
errno = 0;
1361613641
len_written = fwrite (buf, 1, len, f);
@@ -13755,33 +13780,13 @@ _sim_debug_write_flush (buf, len, FALSE);
1375513780

1375613781
static t_stat _sim_debug_flush (void)
1375713782
{
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-
1376413783
if (sim_deb == NULL) /* no debug? */
1376513784
return SCPE_OK;
1376613785

1376713786
_sim_debug_write_flush ("", 0, TRUE);
13787+
fflush (sim_deb);
13788+
fsync(fileno(sim_deb));
1376813789

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-
}
1378513790
return SCPE_OK;
1378613791
}
1378713792

0 commit comments

Comments
 (0)