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

Lifetime of a Buffer in a C++ addon #3222

Closed
sgerace opened this issue Oct 6, 2015 · 9 comments
Closed

Lifetime of a Buffer in a C++ addon #3222

sgerace opened this issue Oct 6, 2015 · 9 comments
Labels
buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. question Issues that look for answers.

Comments

@sgerace
Copy link

sgerace commented Oct 6, 2015

I'm working on an asynchronous C++ addon that is going to be processing buffers piped in from a stream, and I'm wondering the best way to persist the buffer data from the main event-loop thread into my worker thread. I'm basing much of my design on the way the Node.js zlib module is structured in the sense that I'm exposing a process function of my class that looks something like:

void Processor::ProcessAsync(const FunctionCallbackInfo<Value> &args) {
    Isolate* isolate = args.GetIsolate();

    // Unwrap processor object
    Processor *obj = ObjectWrap::Unwrap<Processor>(args.Holder());

    // Reset callback
    Local<Function> callback = Local<Function>::Cast(args[1]);
    obj->callback_.Reset(isolate, callback);

    // Get current buffer data...if this is set on obj, will is persist?
    unsigned char *buf = (unsigned char *)node::Buffer::Data(args[0]);
    size_t length = node::Buffer::Length(args[0]);

    // Build up the work request
    uv_work_t *request = &(obj->request_);
    uv_queue_work(uv_default_loop(), request, Processor::Process, Processor::After);

    // Return value
    args.GetReturnValue().Set(Undefined(isolate));
}

My question is whether or not the unsigned char *buf will persist throughout the lifetime of my class, or as I suspect, I will need to create a local copy of the data. I'm wondering if there is a way to force v8 to persist the data until I release it, but I'm just getting into the deep-end (or maybe the shallow end...) of interacting with v8 and would appreciate any guidance on how stuff like this is typically accomplished.

@bnoordhuis
Copy link
Member

You need to either keep a reference around to the buffer object or make a copy of the data. It looks like you're already using a v8::Persistent<T> or v8::Global<T> for the callback, I'd just extend that to the buffer object.

@sgerace
Copy link
Author

sgerace commented Oct 6, 2015

Ahh, yeah, makes sense. So in my class something like:

v8::Persistent<v8::Object> buffer_;
v8::Persistent<v8::Function> callback_; // I already have this one

And then set it in my function the same way I'm setting the callback:

obj->buffer_.Reset(isolate, args[0]);
obj->callback_.Reset(isolate, callback);

Am I then correct in assuming that when my class's destructor is called these two persistent references are both released, so there isn't anything else I would need to do?

Now that I know where to look, it seems obvious; I'm guessing that I can also make a call to buffer_.Reset() (with no arguments) in my class' cleanup function (to force cleanup independent of GC) to release the handles without having to rely on my destructor being called, correct?

@bnoordhuis
Copy link
Member

v8::Persistent<T> doesn't Reset() in its destructor (by default) but v8::Global<T> does. If you want to play it safe, call Reset() explicitly in your destructor.

@mscdex mscdex added buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. question Issues that look for answers. labels Oct 6, 2015
@sgerace
Copy link
Author

sgerace commented Oct 7, 2015

Ahh, yep, once again, knowing what to look for helps. For future reference, I found the relevant section in Google's v8 Embedder's Guide: https://developers.google.com/v8/embed#handles-and-garbage-collection

Specifically where they state:

A Persistent<SomeType> can be constructed with its constructor, but must be explicitly cleared with Persistent::Reset.

Thanks for the help!

@sgerace sgerace closed this as completed Oct 7, 2015
@sgerace
Copy link
Author

sgerace commented Oct 7, 2015

@bnoordhuis Upon further reading, I found this comment in the same guide:

During the garbage collection process the garbage collector often moves objects to different locations in the heap. When the garbage collector moves an object the garbage collector also updates all handles that refer to the object with the object's new location.

Seeing as though I cannot interact with v8 in my uv worker function, do I need to be concerned about my data moving if I store a persistent handle at my class level and then pass a pointer to my data into my worker function? In other words, does the persistent handle guarantee that the pointer I get from node::Buffer::Data will remain valid?

@sgerace sgerace reopened this Oct 7, 2015
@bnoordhuis
Copy link
Member

The memory that the buffer object points to won't move but the object itself can. Rule of thumb: don't interact with node or V8 when on a different thread. That means you shouldn't call e.g. node::Buffer::Data() from the thread pool.

@sgerace
Copy link
Author

sgerace commented Oct 7, 2015

Makes sense, so by that logic if I store a pointer to the data (i.e., as a char * obtained from node::Buffer::Data()) in my class, it should be safe to consume that data in a worker thread as long as the Buffer itself is contained in a Persistent handle (also as a class variable). Does that sound correct?

@bnoordhuis
Copy link
Member

Yes, 100% correct.

@sgerace
Copy link
Author

sgerace commented Oct 7, 2015

@bnoordhuis Awesome, thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

3 participants