Repository navigation
Buffer.from not support SharedArrayBuffer #8440
Description
Activity
I’ve labelled this
good first contributionand would be happy to help anyone who wants to have a go at this!Reacted by F. Hinkelmann and James M SnellGood catch on this. I'd been meaning to give this a try but hadn't gotten around to it.
/cc @nodejs/bufferIsn't it a bit early for this?
Perhaps, but we've taken fixes for other early bits to make sure things work. For example, the SIMD stuff in
util. I wouldn't say that it's a priority but it's worth looking into.The fix is simple. Add a
instanceof SharedArrayBuffercheck. But, without the flag there's no global. Are we supposed to do aif (!global.SharedArrayBuffer) SharedArrayBuffer = nullor some such before it's exposed by default?I would like to help with this issue.
@ojss Awesome! I would maybe wait for #8453 to be merged, or start from there, so that the changes there don’t conflict with yours. That PR should also give a few hints as to where changes are necessary. :)
A couple of hints: You’ll probably need to add one or two lines to
src/node_util.ccforSharedArrayBufferdetection – if you’ve never done anything with C++ before, don’t worry, the changes should be straightforward. Since this feature requires you to run node with a special flag, namely with--harmony-sharedarraybuffer, you can take a look at the top oftest/parallel/test-util-inspect-simd.jsto see how to integrate that into the tests.If you have any questions, feel free to ask here or in #node-dev on Freenode. :)
@addaleax Thank you! I will most certainly requiring help as I still don't completely understand the internals of node.
I saw the PR, it did give me an idea of what I should be doing. But I still don't understand what I must do innode_util.cc. Just add a check to see of the flag is enabled and accordingly set a global?@addaleax Yes I had assumed as much. But what I don't get is where are those functions defined?
The macro assigns a name to a function that is exposed on the JS side right?Am I misunderstanding something?
Is there some documentation that I can refer to?
:)The macro assigns a name to a function that is exposed on the JS side right?
Yeah, that’s what it does. Using macros for that is a bit of magic, but it keeps the code short and makes it easy to change the list.
If you’re asking where these functions pop up – that’s the line at the top of
buffer.jswhich is (currently)const { isArrayBuffer } = process.binding('util');, where you may need to addisSharedArrayBuffer, too.Is there some documentation that I can refer to?
Not sure what you mean – there aren’t any docs for internal stuff like that in
node_util.ccright now…Well looks like I got a few things correct.
Yeah, that’s what it does. Using macros for that is a bit of magic, but it keeps the code short and makes it easy to change the list.
I am sorry but I still don't understand where the definition of the function is? Is it generated on the fly?
9 remaining items
@ojss The
Flags:line should probably come first in the test file, you can useassert.deepStrictEqual(buf1, buf2)instead of an extraBuffer.compare, and you may want to add tests forBuffer.byteLength, which should also acceptSharedArrayBuffers.But generally, yeah, it looks like you’re ready for opening a pull request – it’s much easier to comment on specific lines, run the tests, etc. that way.
Maybe one more thing: If you want to propose a change that closes an issue like this one, you can add a line like
Fixes: https://github.com/nodejs/node/issues/8440to your git commit message :)@addaleax @ojss sorry,the patch above is just a demo, it is not completed yet.Because it is not handle the length of Buffer.from(arraybuffer,length).
let sab =new SharedArrayBuffer(24);
const arr = new Uint16Array(sab);arr[0] = 5000;
arr[1] = 4000;// Shares memory with arr
const buf = Buffer.from(arr.buffer,10);this would ignore the length 10 and make buffer from the whole arr.buffer.
@wlunlimited If you see the definition of
Buffer.from, you will notice that at the end there is a call tofromObject().fromObjectonly takes the object and not the length. So to account for the length we would have to change the signature offromObject.or I could make another function similar to
fromArrayBuffer, in fact maybe even getfromArrayBufferto handle theSharedArrayBuffer? Is that possible @addaleax?I will be opening a pull request in some time.
- added a commit that references this issue
on Sep 17, 2016 - added a commit that references this issue
on Oct 11, 2016
create a buffer from SharedArrayBuffer just like the buff.from sample in doc:
let sab =new SharedArrayBuffer(24);
const arr = new Uint16Array(sab);
arr[0] = 5000;
arr[1] = 4000;
// Shares memory with
arrconst buf = Buffer.from(arr.buffer);
// Prints: <Buffer 88 13 a0 0f>
console.log(buf);
// Changing the original Uint16Array changes the Buffer also
arr[1] = 6000;
// Prints: <Buffer 88 13 70 17>
console.log(buf);
run above code :
node --harmony-sharedarraybuffer ./test.js
and then we would got a error below:
buffer.js:259
throw new TypeError(kFromErrorMsg);
^
TypeError: First argument must be a string, Buffer, ArrayBuffer, Array, or array-like object.
at fromObject (buffer.js:259:9)
at Function.Buffer.from (buffer.js:96:10)
at Object. (/home/Myprojects/node_git/test_buff_sab.js:8:20)
at Module._compile (module.js:556:32)
at Object.Module._extensions..js (module.js:565:10)
at Module.load (module.js:473:32)
at tryModuleLoad (module.js:432:12)
at Function.Module._load (module.js:424:3)
at Module.runMain (module.js:590:10)
at run (bootstrap_node.js:394:7)