actualClose can close fd while writing

[ronag]

actualClose can close fd while writing causing undefined behavior.

[ronag]

I think actualClose just needs a:

  if (sonic._writing) {
    sonic.once('drain', actualClose.bind(null, sonic))
    return
  }

[mcollina]

Did you see this happening?

[ronag]

Did you see this happening?

Nope. Just based on review of the code. These kind of undefined behaviors are very to actually notice in practice.

[mcollina]

This is protected and it can't happen.

[ronag]

I'm not sure I agree. It only waits for it to be ready not to finish pending async writes (writing).

[ronag]

If you add the following assertion then the test suite will fail:

index 45bf86d..2d4f8a2 100644
--- a/index.js
+++ b/index.js
@@ -8,6 +8,7 @@ const path = require('path')
 const BUSY_WRITE_TIMEOUT = 100
 
 const sleep = require('atomic-sleep')
+const assert = require('assert')
 
 // 16 MB - magic number
 // This constant ensures that SonicBoom only needs
@@ -368,6 +369,7 @@ function actualClose (sonic) {
     sonic.once('ready', actualClose.bind(null, sonic))
     return
   }
+  assert(!sonic._writing)
   // TODO write a test to check if we are not leaking fds
   fs.close(sonic.fd, (err) => {
     if (err) {

[mcollina]

ah sorry! I ready 'drain' for 'ready'.

In practice I have not seen this being a problem - we have done extensive testing and we did not get corrupt data.