Skip to content
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

Pointer#toBuffer throws "TypeError: First argument must be a Buffer" #48

Closed
heroboy opened this issue Apr 28, 2012 · 5 comments
Closed

Comments

@heroboy
Copy link

heroboy commented Apr 28, 2012

This doesn't occur everytime, seldom.
Here is the code:

function PointerToBuffer(ptr,length)
{
    try
    {
        return ptr.toBuffer(length);
    }
    catch(e)
    {
        console.log("ptr=" + ptr);
        console.log("len=" + length);
        console.dir(ptr);
        throw e;
    }
}

and output:

ptr=[object Pointer]
len=75
<Pointer address="475304984" allocated="8392" free="true">
[jsbs1126]onData: catch error,TypeError: First argument must be a Buffer

D:\project\nodehall\GameLink.js:291
                                throw e;
          ^
TypeError: First argument must be a Buffer
    at new Buffer (buffer.js:266:14)
    at PointerToBuffer (D:\project\nodehall\NetLink.js:69:14)
    at Object.decryptCmd (D:\project\nodehall\NetLink.js:222:37)
    at Object.tryParse (D:\project\nodehall\NetLink.js:226:15)
    at GameLink.onData (D:\project\nodehall\GameLink.js:279:25)
    at Socket.<anonymous> (D:\project\nodehall\GameLink.js:141:55)
    at Socket.emit (events.js:67:17)
    at TCP.onread (net.js:329:14)
@TooTallNate
Copy link
Member

Can you give me a more complete example that reproduces the error?

@heroboy
Copy link
Author

heroboy commented May 3, 2012

var ffi = require("./node-ffi");
var arr = [];
for(i=0;i<1000000;++i)
{
    var ptr = new ffi.Pointer(100+i%1000);
    ptr.toBuffer();
    arr.push([ptr,ptr.toBuffer()]);
}

output

TypeError: First argument must be a Buffer
    at new Buffer (buffer.js:266:14)
    at Object.<anonymous> (D:\project\nodehall\test.js:6:6)
    at Module._compile (module.js:432:26)
    at Object..js (module.js:450:10)
    at Module.load (module.js:351:31)
    at Function._load (module.js:310:12)
    at Array.0 (module.js:470:10)
    at EventEmitter._tickCallback (node.js:192:40)

I think create Buffer in c++,you should use String::NewSymbol("Buffer"),not String::New("Buffer").
I write my own wrapper of my c++ functions.
Here is my function to create Buffer:

Handle<Value> NewNodeBuffer(void * buf,int len)
{
    v8::HandleScope scope;
    node::Buffer * slowBuffer = node::Buffer::New((char*)buf,len);
    assert(!slowBuffer->handle_.IsEmpty());
    Local<Object> global = v8::Context::GetCurrent()->Global();
    Local<Value> bv = global->Get(String::NewSymbol("Buffer"));
    assert(bv->IsFunction());
    Local<Function> b = Local<Function>::Cast(bv);
    Handle<Value> argv[3] = { slowBuffer->handle_, Integer::New(len) , Integer::New(0) };
    Handle<Object> instance = b->NewInstance(3, argv);
    return scope.Close(instance);
}

If I change NewSymbol to New,it will crash after my program runs a while.

@TooTallNate
Copy link
Member

Interesting. Indeed, your script crashes for me, and the proposed fix fixes that. But I've come to realize that there's absolutely no benefit in doing the "SlowBuffer -> Buffer" dance, so I'm gonna just return the SlowBuffer instance directly.

@santigimeno
Copy link

Hi,

Sorry to hijack this thread, but I was looking for information about the "SlowBuffer->Buffer dance" and found your comment about not having any benefit. Why is that? Is this a general comment or it only applies to this commit?

Thanks!

@TooTallNate
Copy link
Member

@santigimeno It applies to any time you create a SlowBuffer instance, which is what we were doing already. Wrapping it in a JS-buffer doesn't magically make it any faster, it's just another object wrapping the SlowBuffer we already created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants