Skip to content

Commit

Permalink
Fix out of memory (pinojs#28)
Browse files Browse the repository at this point in the history
* Fix out of memory crash when writing 1GB+.

* Updated dependencies

* Do not run oom tests on Travis

* Maybe a pass?

* boolean logic is tricky

* custom CI run to debug issues

* hould be fixed now

* Run big tests on travis
  • Loading branch information
mcollina authored Jul 22, 2019
1 parent c96197b commit 6dae985
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 4 deletions.
14 changes: 13 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ const EventEmitter = require('events')
const flatstr = require('flatstr')
const inherits = require('util').inherits

// 16 MB - magic number
// This constant ensures that SonicBoom only needs
// 32 MB of free memory to run. In case of having 1GB+
// of data to write, this prevents an out of memory
// condition.
const MAX_WRITE = 16 * 1024 * 1024

function openFile (file, sonic) {
sonic._opening = true
sonic._writing = true
Expand Down Expand Up @@ -246,7 +253,12 @@ function actualWrite (sonic) {
sonic._writing = true
var buf = sonic._buf
var release = sonic.release
sonic._buf = ''
if (buf.length > MAX_WRITE) {
buf = buf.slice(0, MAX_WRITE)
sonic._buf = sonic._buf.slice(MAX_WRITE)
} else {
sonic._buf = ''
}
flatstr(buf)
sonic._writingBuf = buf
if (sonic.sync) {
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"description": "Extremely fast utf8 only stream implementation",
"main": "index.js",
"scripts": {
"test": "standard | snazzy && tap test.js "
"test": "standard | snazzy && tap --no-esm -t 60 test.js"
},
"repository": {
"type": "git",
Expand All @@ -30,10 +30,10 @@
"pre-commit": "^1.2.2",
"proxyquire": "^2.1.0",
"snazzy": "^8.0.0",
"standard": "^12.0.0",
"standard": "^13.0.2",
"tap": "^12.1.0"
},
"dependencies": {
"flatstr": "^1.0.9"
"flatstr": "^1.0.12"
}
}
60 changes: 60 additions & 0 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -614,3 +614,63 @@ test('sync writing is fully sync', (t) => {
const data = fs.readFileSync(dest, 'utf8')
t.equal(data, 'hello world\nsomething else\n')
})

// These they will fail on Node 6, as we cannot allocate a string this
// big. It's considered a won't fix on Node 6, as it's deprecated.
if (process.versions.node.indexOf('6.') !== 0) {
test('write enormously large buffers async', (t) => {
t.plan(3)

const dest = file()
const fd = fs.openSync(dest, 'w')
const stream = new SonicBoom(fd, 0, false)

const buf = Buffer.alloc(1024).fill('x').toString() // 1 MB
let length = 0

for (let i = 0; i < 1024 * 1024; i++) {
length += buf.length
stream.write(buf)
}

stream.end()

stream.on('finish', () => {
fs.stat(dest, (err, stat) => {
t.error(err)
t.equal(stat.size, length)
})
})
stream.on('close', () => {
t.pass('close emitted')
})
})

test('write enormously large buffers sync', (t) => {
t.plan(3)

const dest = file()
const fd = fs.openSync(dest, 'w')
const stream = new SonicBoom(fd, 0, true)

const buf = Buffer.alloc(1024).fill('x').toString() // 1 MB
let length = 0

for (let i = 0; i < 1024 * 1024; i++) {
length += buf.length
stream.write(buf)
}

stream.end()

stream.on('finish', () => {
fs.stat(dest, (err, stat) => {
t.error(err)
t.equal(stat.size, length)
})
})
stream.on('close', () => {
t.pass('close emitted')
})
})
}

0 comments on commit 6dae985

Please sign in to comment.