Skip to content

AsyncWorker: introduce Destroy() method#488

Closed
gabrielschulhof wants to merge 1 commit intonodejs:masterfrom
gabrielschulhof:async-worker-destruct
Closed

AsyncWorker: introduce Destroy() method#488
gabrielschulhof wants to merge 1 commit intonodejs:masterfrom
gabrielschulhof:async-worker-destruct

Conversation

@gabrielschulhof
Copy link
Copy Markdown
Contributor

AsyncWorker contained the assumption that instances of its subclasses
were allocated using new, because it unconditionally destroyed
instances using delete.

This change replaces the call to delete with a call to a protected
instance method Destroy(), which can be overridden by subclasses.
This ensures that users can employ their own allocators when
creating AsyncWorker subclass instances because they can override
the Destroy() method to use their deallocator of choice.

Re: #231 (comment)

`AsyncWorker` contained the assumption that instances of its subclasses
were allocated using `new`, because it unconditionally destroyed
instances using `delete`.

This change replaces the call to `delete` with a call to a protected
instance method `Destroy()`, which can be overridden by subclasses.
This ensures that users can employ their own allocators when
creating `AsyncWorker` subclass instances because they can override
the `Destroy()` method to use their deallocator of choice.

Re: nodejs#231 (comment)
Copy link
Copy Markdown
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@gabrielschulhof
Copy link
Copy Markdown
Contributor Author

gabrielschulhof commented Jun 11, 2019

gabrielschulhof pushed a commit that referenced this pull request Jun 11, 2019
`AsyncWorker` contained the assumption that instances of its subclasses
were allocated using `new`, because it unconditionally destroyed
instances using `delete`.

This change replaces the call to `delete` with a call to a protected
instance method `Destroy()`, which can be overridden by subclasses.
This ensures that users can employ their own allocators when
creating `AsyncWorker` subclass instances because they can override
the `Destroy()` method to use their deallocator of choice.

Re: #231 (comment)
PR-URL: #488
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@gabrielschulhof
Copy link
Copy Markdown
Contributor Author

Landed in 3b6b9eb.

@gabrielschulhof gabrielschulhof deleted the async-worker-destruct branch June 11, 2019 19:41
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
`AsyncWorker` contained the assumption that instances of its subclasses
were allocated using `new`, because it unconditionally destroyed
instances using `delete`.

This change replaces the call to `delete` with a call to a protected
instance method `Destroy()`, which can be overridden by subclasses.
This ensures that users can employ their own allocators when
creating `AsyncWorker` subclass instances because they can override
the `Destroy()` method to use their deallocator of choice.

Re: nodejs/node-addon-api#231 (comment)
PR-URL: nodejs/node-addon-api#488
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
`AsyncWorker` contained the assumption that instances of its subclasses
were allocated using `new`, because it unconditionally destroyed
instances using `delete`.

This change replaces the call to `delete` with a call to a protected
instance method `Destroy()`, which can be overridden by subclasses.
This ensures that users can employ their own allocators when
creating `AsyncWorker` subclass instances because they can override
the `Destroy()` method to use their deallocator of choice.

Re: nodejs/node-addon-api#231 (comment)
PR-URL: nodejs/node-addon-api#488
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
`AsyncWorker` contained the assumption that instances of its subclasses
were allocated using `new`, because it unconditionally destroyed
instances using `delete`.

This change replaces the call to `delete` with a call to a protected
instance method `Destroy()`, which can be overridden by subclasses.
This ensures that users can employ their own allocators when
creating `AsyncWorker` subclass instances because they can override
the `Destroy()` method to use their deallocator of choice.

Re: nodejs/node-addon-api#231 (comment)
PR-URL: nodejs/node-addon-api#488
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
`AsyncWorker` contained the assumption that instances of its subclasses
were allocated using `new`, because it unconditionally destroyed
instances using `delete`.

This change replaces the call to `delete` with a call to a protected
instance method `Destroy()`, which can be overridden by subclasses.
This ensures that users can employ their own allocators when
creating `AsyncWorker` subclass instances because they can override
the `Destroy()` method to use their deallocator of choice.

Re: nodejs/node-addon-api#231 (comment)
PR-URL: nodejs/node-addon-api#488
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
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

Successfully merging this pull request may close these issues.

2 participants