Skip to content

Commit 7df5086

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, macOS invoke fsync(). Windows invokes a fsync() wrapper that invokes _commit(). Issue traced to _sim_debug_flush() calling sim_set_deboff(0, NULL) to simulate fsync().
1 parent b036821 commit 7df5086

File tree

1 file changed

+47
-11
lines changed

1 file changed

+47
-11
lines changed

scp.c

+47-11
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,22 @@
253253
#include ".git-commit-id.h"
254254
#endif
255255

256+
/* fsync() and _commit(). Update the preprocessor conditional below for add'l
257+
* platforms. Minimally supports Linux and macOS. */
258+
#if _BSD_SOURCE || _XOPEN_SOURCE || _POSIX_C_SOURCE >= 200112L
259+
#define HAVE_FSYNC
260+
#include <unistd.h>
261+
#elif defined(_WIN32) || defined(_WIN64)
262+
#define HAVE_FSYNC
263+
#include <io.h>
264+
265+
/* fsync() wrapper for Windows. */
266+
static inline int fsync(int fd)
267+
{
268+
return _commit(fd);
269+
}
270+
#endif
271+
256272
#ifndef MAX
257273
#define MAX(a,b) (((a) >= (b)) ? (a) : (b))
258274
#endif
@@ -13611,6 +13627,16 @@ static void _debug_fwrite_all (const char *buf, size_t len, FILE *f)
1361113627
{
1361213628
size_t len_written;
1361313629

13630+
/* NOTE: This is UNSAFE in a multithreaded SIMH if not using fsync() or _commit().
13631+
*
13632+
* _sim_debug_flush() calls sim_set_deboff(0, NULL), which NULLs sim_deb (aka f).
13633+
* This causes a race on sim_deb between threads -- a thread can read a NULL sim_deb
13634+
* and call _debug_fwrite_all() with f == NULL.
13635+
*
13636+
* f == NULL: fwrite() returns 0 on Linux and Windows. len never gets decremented
13637+
* and an infinite loop ensues (cue "Forever NULL" sung to the tune of Alphaville's
13638+
* "Forever Young".)
13639+
*/
1361413640
while (len > 0) {
1361513641
errno = 0;
1361613642
len_written = fwrite (buf, 1, len, f);
@@ -13755,23 +13781,31 @@ _sim_debug_write_flush (buf, len, FALSE);
1375513781

1375613782
static t_stat _sim_debug_flush (void)
1375713783
{
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-
1376413784
if (sim_deb == NULL) /* no debug? */
1376513785
return SCPE_OK;
1376613786

1376713787
_sim_debug_write_flush ("", 0, TRUE);
13788+
fflush (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-
13790+
#if defined(HAVE_FSYNC)
13791+
/* Linux, macOS: Use fsync() to flush sim_deb contents and metadata. Windows has
13792+
* equivalent functionality in an inline wrapper. */
13793+
fsync(fileno(sim_deb));
13794+
#else
1377413795
if (!(saved_deb_switches & SWMASK ('B'))) {
13796+
/* Simulated fsync(). This cannot be used in a multithreaded SIMH because NULL-ing
13797+
* sim_deb in sim_set_deboff() can cause another thread to infinitely loop in
13798+
* _debug_fwrite_all().
13799+
*
13800+
* N.B. _sim_debug_flush() is eventually called by flush_svc() on 30 second
13801+
* intervals. */
13802+
13803+
int32 saved_quiet = sim_quiet;
13804+
int32 saved_sim_switches = sim_switches;
13805+
int32 saved_deb_switches = sim_deb_switches;
13806+
struct timespec saved_deb_basetime = sim_deb_basetime;
13807+
char saved_debug_filename[CBUFSIZE];
13808+
1377513809
strcpy (saved_debug_filename, sim_logfile_name (sim_deb, sim_deb_ref));
1377613810

1377713811
sim_quiet = 1;
@@ -13782,6 +13816,8 @@ if (!(saved_deb_switches & SWMASK ('B'))) {
1378213816
sim_switches = saved_sim_switches;
1378313817
sim_quiet = saved_quiet;
1378413818
}
13819+
#endif
13820+
1378513821
return SCPE_OK;
1378613822
}
1378713823

0 commit comments

Comments
 (0)