This repository has been archived by the owner. It is now read-only.

fs.read and fs.write break oddly when the same cb is used repeatedly #814

Closed
isaacs opened this Issue Mar 19, 2011 · 6 comments

Comments

Projects
None yet
3 participants
@isaacs

isaacs commented Mar 19, 2011

This is a weird race condition:

var buff1 = new Buffer(1000), buff2 = new Buffer(10);
function cb () {}
fs.read(fd, buff1, 0, 1000, 0, cb)
fs.read(someOtherFd, buff2, 0, 10, cb)

Because src/node_file.cc does this: https://github.com/joyent/node/blob/master/src/node_file.cc#L716-718 there are going to be some cases where cb.buffer is set to buff1 when handling the 10 bytes of data from someOtherFd, or much worse, buff2 when trying to read in the 1000 bytes from fd.

Solution: instead of passing the callback handle as ev_data, pass a struct that looks like:

{ callback: Persistent<Function>,
  buffer: Buffer }

and then ev_ref and ev_unref the buffer if not null.

@ry

This comment has been minimized.

Show comment Hide comment
@xk

This comment has been minimized.

Show comment Hide comment
@xk

xk Mar 23, 2011

xk commented Mar 23, 2011

@xk

This comment has been minimized.

Show comment Hide comment
@xk

This comment has been minimized.

Show comment Hide comment
@xk

xk Mar 28, 2011

Retain buffers in fs.read/write()

Closed by e7604b1.
Closed by e7604b1.

xk commented Mar 28, 2011

Retain buffers in fs.read/write()

Closed by e7604b1.
Closed by e7604b1.

@ry ry closed this Mar 28, 2011

@ry

This comment has been minimized.

Show comment Hide comment
@ry

ry Mar 28, 2011

thanks jorge landed with some minor changes

ry commented Mar 28, 2011

thanks jorge landed with some minor changes

@xk

This comment has been minimized.

Show comment Hide comment
@xk

xk Mar 29, 2011

You're welcome. Have to fix the docs yet.

xk commented Mar 29, 2011

You're welcome. Have to fix the docs yet.

coolaj86 pushed a commit that referenced this issue Apr 15, 2011

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.