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

doc: add documentation for AtExit hook #1014

Closed
wants to merge 5 commits into from
Closed

doc: add documentation for AtExit hook #1014

wants to merge 5 commits into from

Conversation

JCMais
Copy link
Contributor

@JCMais JCMais commented Mar 1, 2015

See #999.


Registers exit hooks that run after the event loop has ended, but before the VM is killed.

Callbacks are run in reverse order of registration, i.e. newest first. AtExit takes callback and its arguments as arguments.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The explanation here seems confusing, suggestions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Callbacks are run in last-in, first-out order. AtExit takes two parameters: a pointer to a callback function to run at exit, and a pointer to untyped context data to be passed to that callback."

@chrisdickinson
Copy link
Contributor

I'm generally +1 on this – we might see if it breaks the addons tests (which generate tests from code snippets in the addons.markdown doc.)

}

void init(Handle<Object> exports) {
AtExit(at_exit_cb1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe AtExit(at_exit_cb1, exports->GetIsolate() and then static_cast<v8::Isolate*>(arg) in the callback? v8::Isolate::GetCurrent() is going away someday so I'd prefer not to encourage its use in the documentation.

(That said, I realize it's used in plenty of other places in this document. I'll file a PR to clean that up.)

@bnoordhuis
Copy link
Member

@JCMais Can you make your sample conform to the style introduced in #1125 and wrap lines at 80 columns? Thanks.

@mscdex mscdex added the doc Issues and PRs related to the documentations. label Mar 22, 2015
@Fishrock123
Copy link
Member

Ping @JCMais, are you able to update?

OpenSourceSteve and others added 5 commits April 4, 2015 20:20
Updates addons.markdown to mention AtExit() function.  Addresses issue #8324.
Updated description, added arguments and arguments types, and quick code example.
Incorporated latest suggestions.
@JCMais
Copy link
Contributor Author

JCMais commented Apr 4, 2015

@Fishrock123 Sorry, totally forgot about this pull request. Updated.

@JCMais JCMais changed the title Add documentation for AtExit hook doc: add documentation for AtExit hook Apr 10, 2015
@brendanashworth
Copy link
Contributor

ping @bnoordhuis for review?

bnoordhuis pushed a commit that referenced this pull request May 26, 2015
Fixes: #999
PR-URL: #1014
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@bnoordhuis
Copy link
Member

Landed in 98649fd with some fixes to make make test-addons pass. Cheers.

@bnoordhuis bnoordhuis closed this May 26, 2015
This was referenced May 27, 2015
andrewdeandrade pushed a commit to andrewdeandrade/node that referenced this pull request Jun 3, 2015
Fixes: nodejs/node#999
PR-URL: nodejs/node#1014
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants