Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
Dgram buffers#13623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Dgram buffers #13623
Changes from all commits
9d6cd42b84bf958aed7aaf7c1aa6a99ec112581c09b0ac4fff6bf7f2d5789c0cd79a58de458d1f196be9f4d3564File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -227,6 +227,20 @@ never have reason to call this. | ||
| If `multicastInterface` is not specified, the operating system will attempt to | ||
| drop membership on all valid interfaces. | ||
| ### socket.getRecvBufferSize(size) | ||
| <!-- YAML | ||
| added: REPLACEME | ||
| --> | ||
| * Returns{number} the `SO_RCVBUF` socket receive buffer size in bytes. | ||
| ### socket.getSendBufferSize(size) | ||
| <!-- YAML | ||
| added: REPLACEME | ||
| --> | ||
| * Returns{number} the `SO_SNDBUF` socket send buffer size in bytes. | ||
| ### socket.ref() | ||
| <!-- YAML | ||
| added: v0.9.1 | ||
| @@ -398,6 +412,26 @@ decremented to 0 by a router, it will not be forwarded. | ||
| The argument passed to to `socket.setMulticastTTL()` is a number of hops | ||
| between 0 and 255. The default on most systems is `1` but can vary. | ||
| ### socket.setRecvBufferSize(size) | ||
| <!-- YAML | ||
| ||
| added: REPLACEME | ||
| --> | ||
| * `size`{number} Integer | ||
| Sets the `SO_RCVBUF` socket option. Sets the maximum socket receive buffer | ||
| in bytes. | ||
| ### socket.setSendBufferSize(size) | ||
| <!-- YAML | ||
| added: REPLACEME | ||
| --> | ||
| * `size`{number} Integer | ||
| Sets the `SO_SNDBUF` socket option. Sets the maximum socket send buffer | ||
| in bytes. | ||
| ### socket.setTTL(ttl) | ||
| <!-- YAML | ||
| added: v0.1.101 | ||
| @@ -461,6 +495,9 @@ changes: | ||
| - version: REPLACEME | ||
| pr-url: https://github.com/nodejs/node/pull/14560 | ||
| description: The `lookup` option is supported. | ||
| - version: REPLACEME | ||
| pr-url: https://github.com/nodejs/node/pull/13623 | ||
| description: `recvBufferSize` and `sendBufferSize` options are supported now. | ||
| --> | ||
| * `options`{Object} Available options are: | ||
| @@ -469,6 +506,8 @@ changes: | ||
| * `reuseAddr`{boolean} When `true` [`socket.bind()`][] will reuse the | ||
| address, even if another process has already bound a socket on it. Optional. | ||
| Defaults to `false`. | ||
| * `recvBufferSize`{number} - Optional. Sets the `SO_RCVBUF` socket value. | ||
| * `sendBufferSize`{number} - Optional. Sets the `SO_SNDBUF` socket value. | ||
| * `lookup`{Function} Custom lookup function. Defaults to [`dns.lookup()`][]. | ||
| Optional. | ||
| * `callback`{Function} Attached as a listener for `'message'` events. Optional. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -46,6 +46,7 @@ using v8::Object; | ||
| using v8::PropertyAttribute; | ||
| using v8::PropertyCallbackInfo; | ||
| using v8::String; | ||
| using v8::Uint32; | ||
| using v8::Undefined; | ||
| using v8::Value; | ||
| @@ -134,6 +135,7 @@ void UDPWrap::Initialize(Local<Object> target, | ||
| env->SetProtoMethod(t, "setMulticastLoopback", SetMulticastLoopback); | ||
| env->SetProtoMethod(t, "setBroadcast", SetBroadcast); | ||
| env->SetProtoMethod(t, "setTTL", SetTTL); | ||
| env->SetProtoMethod(t, "bufferSize", BufferSize); | ||
| env->SetProtoMethod(t, "ref", HandleWrap::Ref); | ||
| env->SetProtoMethod(t, "unref", HandleWrap::Unref); | ||
| @@ -222,6 +224,43 @@ void UDPWrap::Bind6(const FunctionCallbackInfo<Value>& args){ | ||
| } | ||
| void UDPWrap::BufferSize(const FunctionCallbackInfo<Value>& args){ | ||
| Environment* env = Environment::GetCurrent(args); | ||
| UDPWrap* wrap; | ||
| ASSIGN_OR_RETURN_UNWRAP(&wrap, | ||
| args.Holder(), | ||
| args.GetReturnValue().Set(UV_EBADF)); | ||
| CHECK(args[0]->IsUint32()); | ||
| ||
| CHECK(args[1]->IsUint32()); | ||
| int size = static_cast<int>(args[0].As<Uint32>()->Value()); | ||
| ||
| if (size != args[0].As<Uint32>()->Value()){ | ||
| if (args[1].As<Uint32>()->Value() == 0) | ||
| return env->ThrowUVException(EINVAL, "uv_recv_buffer_size"); | ||
| else | ||
| return env->ThrowUVException(EINVAL, "uv_send_buffer_size"); | ||
| } | ||
| int err; | ||
| if (args[1].As<Uint32>()->Value() == 0){ | ||
| err = uv_recv_buffer_size(reinterpret_cast<uv_handle_t*>(&wrap->handle_), | ||
| &size); | ||
| } else{ | ||
| err = uv_send_buffer_size(reinterpret_cast<uv_handle_t*>(&wrap->handle_), | ||
| &size); | ||
| } | ||
| if (err != 0){ | ||
| if (args[1].As<Uint32>()->Value() == 0) | ||
| return env->ThrowUVException(err, "uv_recv_buffer_size"); | ||
| else | ||
| return env->ThrowUVException(err, "uv_send_buffer_size"); | ||
| } | ||
| args.GetReturnValue().Set(size); | ||
| } | ||
| #define X(name, fn) \ | ||
| void UDPWrap::name(const FunctionCallbackInfo<Value>& args){\ | ||
| UDPWrap* wrap = Unwrap<UDPWrap>(args.Holder()); \ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| 'use strict' | ||
| const common = require('../common'); | ||
| const assert = require('assert'); | ||
| const dgram = require('dgram'); | ||
| { | ||
| // Should throw error if the socket is never bound. | ||
| const errorObj ={ | ||
| code: 'ERR_SOCKET_BUFFER_SIZE', | ||
| type: Error, | ||
| message: /^Could not get or set buffer size:.*$/ | ||
| }; | ||
| const socket = dgram.createSocket('udp4'); | ||
| assert.throws(() =>{ | ||
| socket.setRecvBufferSize(8192); | ||
| }, common.expectsError(errorObj)); | ||
| assert.throws(() =>{ | ||
| socket.setSendBufferSize(8192); | ||
| }, common.expectsError(errorObj)); | ||
| assert.throws(() =>{ | ||
| socket.getRecvBufferSize(); | ||
| }, common.expectsError(errorObj)); | ||
| assert.throws(() =>{ | ||
| socket.getSendBufferSize(); | ||
| }, common.expectsError(errorObj)); | ||
| } | ||
| { | ||
| // Should throw error if invalid buffer size is specified | ||
| const errorObj ={ | ||
| code: 'ERR_SOCKET_BAD_BUFFER_SIZE', | ||
| type: TypeError, | ||
| message: /^Buffer size must be a positive integer$/ | ||
| }; | ||
| const badBufferSizes = [-1, Infinity, 'Doh!']; | ||
| const socket = dgram.createSocket('udp4'); | ||
| socket.bind(common.mustCall(() =>{ | ||
| badBufferSizes.forEach((badBufferSize) =>{ | ||
| assert.throws(() =>{ | ||
| socket.setRecvBufferSize(badBufferSize); | ||
| }, common.expectsError(errorObj)); | ||
| assert.throws(() =>{ | ||
| socket.setSendBufferSize(badBufferSize); | ||
| }, common.expectsError(errorObj)); | ||
| }); | ||
| socket.close(); | ||
| })); | ||
| } | ||
| { | ||
| // Can set and get buffer sizes after binding the socket. | ||
| const socket = dgram.createSocket('udp4'); | ||
| socket.bind(common.mustCall(() =>{ | ||
| socket.setRecvBufferSize(10000); | ||
| socket.setSendBufferSize(10000); | ||
| // note: linux will double the buffer size | ||
| const expectedBufferSize = common.isLinux ? 20000 : 10000; | ||
| assert.strictEqual(socket.getRecvBufferSize(), expectedBufferSize); | ||
| assert.strictEqual(socket.getSendBufferSize(), expectedBufferSize); | ||
| socket.close(); | ||
| })); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a thought, but we might not need to mention
SO_RCVBUFandSO_SNDBUFat all. They aren't really needed to work with the API.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are not needed, but I though it was no harm having them mentioned in the docs. I'd like to get a few people's opinion on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that googling
SO_RCVBUFgives very good results (linux man and MSDN) so I'm +0.5There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can always add something like
(see socket(7))if you like, the doctool will expand that into a link.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for the record, I'm not opposed to including them. I just thought we might be able to get by without them. I'm open to whatever everyone thinks is best here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I know, the only reason I included them was that I was familiar with setting the buffer sizes with the JDK, and the javadocs mentioned these property names. When I started working on a small app to get more familiar with javascript, I was goggle'in around "node.js SO_RCVBUF" to see if anyone had already added or was in the process of adding buffer size setting support in node.js.