Skip to content

Commit 38ca4f8

Browse files
authored
WasmFS: Add wasmfs_flush() and use that to get unflushed content checks working (#19448)
We have assertions on unflushed content, which are important as on the Web we only print full lines with console.log(), so printf("hello\nworld"); only prints the first word, and the second waits on a newline before we do a console.log. The assertions see if there is unflushed content and direct the user to how to fix this potentially confusing problem. For the assertion to work, we need to flush not only musl's libc buffers but also our own buffering before console.log. In the JS FS that is done in JS code. For WasmFS, we need to flush them in C++, which this PR adds. This makes that flushing method a public API, since it seems like a generally useful thing.
1 parent 65cfbfa commit 38ca4f8

File tree

5 files changed

+31
-11
lines changed

5 files changed

+31
-11
lines changed

emcc.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2308,6 +2308,9 @@ def phase_linker_setup(options, state, newargs):
23082308
settings.FILESYSTEM = 1
23092309
settings.SYSCALLS_REQUIRE_FILESYSTEM = 0
23102310
settings.JS_LIBRARIES.append((0, 'library_wasmfs.js'))
2311+
if settings.ASSERTIONS:
2312+
# used in assertion checks for unflushed content
2313+
settings.REQUIRED_EXPORTS += ['wasmfs_flush']
23112314
if settings.FORCE_FILESYSTEM:
23122315
# Add exports for the JS API. Like the old JS FS, WasmFS by default
23132316
# includes just what JS parts it actually needs, and FORCE_FILESYSTEM is

src/postamble.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,10 @@ function checkUnflushedContent() {
269269
try { // it doesn't matter if it fails
270270
#if SYSCALLS_REQUIRE_FILESYSTEM == 0 && '$flush_NO_FILESYSTEM' in addedLibraryItems
271271
flush_NO_FILESYSTEM();
272+
#elif WASMFS && hasExportedSymbol('wasmfs_flush')
273+
// In WasmFS we must also flush the WasmFS internal buffers, for this check
274+
// to work.
275+
_wasmfs_flush();
272276
#elif hasExportedSymbol('fflush')
273277
_fflush(0);
274278
#endif

system/include/emscripten/wasmfs.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,15 @@ backend_t wasmfs_create_opfs_backend(void);
6969
backend_t wasmfs_create_icase_backend(backend_constructor_t create_backend,
7070
void* arg);
7171

72+
// Similar to fflush(0), but also flushes all internal buffers inside WasmFS.
73+
// This is necessary because in a Web environment we must buffer at an
74+
// additional level after libc, since console.log() prints entire lines, that
75+
// is, we can't print individual characters as libc feeds them to us, so we
76+
// buffer them and call console.log() only after a newline. This function will
77+
// actually flush all buffers and add newlines as necessary to get everything
78+
// printed out.
79+
void wasmfs_flush(void);
80+
7281
// Hooks
7382

7483
// A hook users can do to create the root directory. Overriding this allows the

system/lib/wasmfs/wasmfs.cpp

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,15 @@ WasmFS::WasmFS() : rootDirectory(initRootDirectory()), cwd(rootDirectory) {
5959
__attribute__((weak)) extern "C" void __lsan_do_leak_check(void) {
6060
}
6161

62+
extern "C" void wasmfs_flush(void) {
63+
// Flush musl libc streams.
64+
fflush(0);
65+
66+
// Flush our own streams. TODO: flush all backends.
67+
(void)SpecialFiles::getStdout()->locked().flush();
68+
(void)SpecialFiles::getStderr()->locked().flush();
69+
}
70+
6271
WasmFS::~WasmFS() {
6372
// See comment above on this function.
6473
//
@@ -69,17 +78,11 @@ WasmFS::~WasmFS() {
6978
// time for the checks to run (since right after this nothing can be printed).
7079
__lsan_do_leak_check();
7180

72-
// Flush musl libc streams.
73-
// TODO: Integrate musl exit() which would call this for us. That might also
74-
// help with destructor priority - we need to happen last.
75-
fflush(0);
76-
77-
// Flush our own streams. TODO: flush all possible streams.
78-
// Note that we lock here, although strictly speaking it is unnecessary given
79-
// that we are in the destructor of WasmFS: nothing can possibly be running
80-
// on files at this time.
81-
(void)SpecialFiles::getStdout()->locked().flush();
82-
(void)SpecialFiles::getStderr()->locked().flush();
81+
// TODO: Integrate musl exit() which would flush the libc part for us. That
82+
// might also help with destructor priority - we need to happen last.
83+
// (But we would still need to flush the internal WasmFS buffers, see
84+
// wasmfs_flush() and the comment on it in the header.)
85+
wasmfs_flush();
8386

8487
// Break the reference cycle caused by the root directory being its own
8588
// parent.

test/test_other.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8985,6 +8985,7 @@ def test_fflush(self):
89858985
# fflush without the full filesystem won't quite work
89868986
self.do_other_test('test_fflush.cpp')
89878987

8988+
@also_with_wasmfs
89888989
def test_fflush_fs(self):
89898990
# fflush with the full filesystem will flush from libc, but not the JS logging, which awaits a newline
89908991
self.do_other_test('test_fflush_fs.cpp', emcc_args=['-sFORCE_FILESYSTEM'])

0 commit comments

Comments
 (0)