Register the 'close' event listener with .once(), not .on().
It doesn't matter in the grand scheme of things because the listener
doesn't keep references to any heavy-weight objects but using .once()
for a oneshot listener is something of a best practice.
This makes it so that `stream.push(chunk)` is the only way to signal the
end of reading, removing the confusing disparity between the
callback-style _read method, and the fact that most real-world streams
do not have a 1:1 corollation between the "please give me data" event,
and the actual arrival of a chunk of data.
It is still possible, of course, to implement a `CallbackReadable` on
top of this. Simply provide a method like this as the callback:
function readCallback(er, chunk) {
if (er)
stream.emit('error', er);
else
stream.push(chunk);
}
However, *only* fs streams actually would behave in this way, so it
makes not a lot of sense to make TCP, TLS, HTTP, and all the rest have
to bend into this uncomfortable paradigm.
A primary motivation of this is to make the onread function more
inline-friendly, but also to make it more easy to explore not having
onread at all, in favor of always using push() to signal the end of
reading.
Don't emit a 'connect' event on sockets that are handed off to
net.Server 'connection' event listeners.
1. It's superfluous because the connection has already been established
at that point.
2. The implementation is arguably wrong because the event is emitted on
the same tick of the event loop while the rule of thumb is to always
emit it on the next one.
This has been tried before in commit f0a440d but was reverted again in
ede1acc because the change was incomplete (at least one test hadn't
been updated).
Fixes#1047 (again).
It's cleaner to only load domain ticker logic when the domains are being
used. This makes execution slightly quicker in both cases, and simpler
from the spinner since there is no need to check if the latest callback
requires use of domains.
Not necessary, since we can handle the error properly on the first tick
now, even if there are event listeners, etc.
Additionally, this removes the unnecessary "_needTickCallback" from
startup, since Module.loadMain() will kick off a nextTick callback right
after it runs the main module.
Fix#4856
This handles the fact that stream.Writable inherits from the Stream class,
meaning that it has the legacy pipe() method. Override that with a pipe()
method that emits an error.
Ensure that Duplex streams ARE still pipe()able, however.
Since the 'readable' flag on streams is sometimes temporary, it's probably
better not to put too much weight on that. But if something is an instanceof
Writable, rather than of Readable or Duplex, then it's safe to say that
reading from it is the wrong thing to do.
Fix#3647
This appears to fix#4673. That bug is very hard to reproduce, so it's
hard to tell for certain, but this approach is more correct anyway.
Hat-tip: @dougwilson
The Readable and Writable classes will nextTick certain things
if in sync mode. The sync flag gets unset after a call to _read
or _write. However, most of these behaviors should also be
deferred until nextTick if no reads have been made (for example,
the automatic '_read up to hwm' behavior on Readable.push(chunk))
Set the sync flag to true in the constructor, so that it will not
trigger an immediate 'readable' event, call to _read, before the
user has had a chance to set a _read method implementation.
Commit 9901b69c introduces a small regression where the trailing base64
padding is no longer written out when Cipher#final is called. Rectify
that.
Fixes#4837.
There are cases where a push() call would return true, even though
the thing being pushed was in fact way way larger than the high
water mark, simply because the 'needReadable' was already set, and
would not get unset until nextTick.
In some cases, this could lead to an infinite loop of pushing data
into the buffer, never getting to the 'readable' event which would
unset the needReadable flag.
Fix by splitting up the emitReadable function, so that it always
sets the flag on this tick, even if it defers until nextTick to
actually emit the event.
Also, if we're not ending or already in the process of reading, it
now calls read(0) if we're below the high water mark. Thus, the
highWaterMark value is the intended amount to buffer up to, and it
is smarter about hitting the target.
It seems like a good idea on the face of it, but lowWaterMarks are
actually not useful, and in practice should always be set to zero.
It would be worthwhile for writers if we actually did some kind of
writev() type of thing, but actually this just delays calling write()
and the overhead of doing a bunch of Buffer copies is not worth the
slight benefit of calling write() fewer times.
lib/path.js:
- throws a TypeError on the filter if the argument is not a string.
test/simple/test-path.js:
- removed the test to check if non-string types are filtered.
- added a test to check if path.join throws TypeError on arguments that
are not strings.
lib/http.js is using stream._handle.readStart/readStop to control
data-flow coming out from underlying stream. If this methods are not
present - data might be buffered regardless of whether it'll be read.
see #4657
Checks have been simplified and optimized for most-used cases.
Calling Buffer with another Buffer as the subject will now use the
SlowBuffer Copy method instead of the for loop.
No need to call for value coercion, just place the ternary inline.
This increases fs.WriteStream throughput dramatically by removing the
"higher default water marks" for fs.WriteStream.
Also includes a benchmark. Current performance is significantly higher
than v0.8 for strings at all tested levels except size=1. Buffer
performance is still lackluster.
Further improvement in the stream.Writable base class is required, but
this is a start.
Prior to v0.10, Node ignored ECONNRESET errors in many situations.
There *are* valid cases in which ECONNRESET should be ignored as a
normal part of the TCP dance, but in many others, it's a very relevant
signal that must be heeded with care.
Exacerbating this problem, if the OutgoingMessage does not have a
req.connection._handle, it assumes that it is in the process of
connecting, and thus buffers writes up in an array.
The problem happens when you reuse a socket between two requests, and it
is destroyed abruptly in between them. The writes will be buffered,
because the socket has no handle, but it's not ever going to GET a
handle, because it's not connecting, it's destroyed.
The proper fix is to treat ECONNRESET correctly. However, this is a
behavior/semantics change, and cannot land in a stable branch.
Fix#4775
Previously, we were only destroying sockets on end if their readable
side had already been ended. This causes a problem for non-readable
streams, since we don't expect to ever see an 'end' event from those.
Treat the lack of a 'readable' flag the same as if it was an ended
readable stream.
Fix#4751
This is causing the CryptoStreams to get into an awful state when
there is a tight loop calling connection.write(chunk) waiting for
a false return.
Because CryptoStreams use read(0) to cycle data, this was causing
the encrypted side to pull way too much data in from the cleartext
side, since the read(0) would make it always call _read.
The unfortunate side effect, fixed in the next patch, is that
CryptoStreams don't automatically cycle when the Socket drains.
Let ECONNRESET network errors bubble up so clients can detect them.
Commit c4454d2e suppressed and turned them into regular end-of-stream
events to fix the then-failing simple/test-regress-GH-1531 test. See
also issue #1571 for (scant) details.
It turns out that special handling is no longer necessary. Remove the
special casing and let the error bubble up naturally.
pummel/test-https-ci-reneg-attack and pummel/test-tls-ci-reneg-attack
are updated because they expected an EPIPE error code that is now an
ECONNRESET. Suppression of the ECONNRESET prevented the test from
detecting that the connection has been severed whereupon the next
write would fail with an EPIPE.
Fixes#1776.
Fix an exception that was raised when the WriteStream was closed
immediately after creating it:
TypeError: Cannot read property 'fd' of undefined
at WriteStream.close (fs.js:1537:18)
<snip>
Avoid the TypeError and make sure the file descriptor is closed.
Fixes#4745.
Otherwise sockets that are 'finish'ed won't be unpiped and `writing to
ended stream` error will arise.
This might sound unrealistic, but it happens in net.js. When
`socket.allowHalfOpen === false`, EOF will cause `.destroySoon()` call which
ends the writable side of net.Socket.
This is more backwards-compatible with stream1 streams like `fs.WriteStream`
which would allow a callback function to be passed in as the only argument.
Closes#4719.
In zlibBuffer(), don't wait for the garbage collector to reclaim the zlib memory
but release it manually. Reduces memory consumption by a factor of 10 or more
with some workloads.
Test case:
function f() {
require('zlib').deflate('xxx', g);
}
function g() {
setTimeout(f, 5);
}
f();
Observe RSS memory usage with and without this commit. After 10,000 iterations,
RSS stabilizes at ~35 MB with this commit. Without, RSS is over 300 MB and keeps
growing.
Cause: whenever the JS object heap hits the high-water mark, the V8 GC sweeps
it clean, then tries to grow it in order to avoid more sweeps in the near
future. Rule of thumb: the bigger the JS heap, the lazier the GC can be.
A side effect of a bigger heap is that objects now live longer. This is harmless
in general but it affects zlib context objects because those are tied to large
buffers that live outside the JS heap, on the order of 16K per context object.
Ergo, don't wait for the GC to reclaim the memory - it may take a long time.
Fixes#4172.
Move the implementation to C++ land. This is similar to commit 3f65916
but this time for the write() function and the Buffer(s, 'hex')
constructor.
Speeds up the benchmark below about 24x (2.6s vs 1:02m).
var s = 'f';
for (var i = 0; i < 26; ++i) s += s; // 64 MB
Buffer(s, 'hex');
Move the implementation to C++ land. The old JS implementation used
string concatenation, was dog slow and consumed copious amounts of
memory for large buffers. Example:
var buf = Buffer(0x1000000); // 16 MB
buf.toString('hex') // Used 3+ GB of memory.
The new implementation operates in O(n) time and space.
Fixes#4700.
Those values, if passed to the _read() cb, will not signal an EOF. Only
null or undefined will mark the end of data, and trigger the end event.
However, great care must be taken if you are returning an empty string
or buffer! There must be some other thing somewhere that will trigger
a read() call, because there will never be a readable event fired later.
This is in preparation for CryptoStreams being ported to streams2, where
it is safe to simply stop reading, because the crypto cycle process will
cause it to read(0) again at some future date.
Ben Noordhuis
isaacs
Trevor Norris
Arianit Uka
Fedor Indutny
Gil Pedersen
Shigeki Ohtsu
Bert Belder
Nathan Rajlich