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

Leaking a current context and a primary context reference unit in scoped_current_device_fallback_t #317

Closed
eyalroz opened this issue Apr 15, 2022 · 0 comments

Comments

@eyalroz
Copy link
Owner

eyalroz commented Apr 15, 2022

We currently (well, actually, up to a few minutes ago) had a resource leak in the scoped_current_device_fallback_t fallback class, which masked the issues in #316 somewhat.

@eyalroz eyalroz added the bug label Apr 15, 2022
@eyalroz eyalroz self-assigned this Apr 15, 2022
eyalroz added a commit that referenced this issue Apr 15, 2022
…t context, primary contexts, and ensuring their existence in various circumstanves:

* Renamed: `context::current::detail_::scoped_current_device_fallback_t` -> `scoped_existence_ensurer_t` `context::current::detail_::scoped_context_existence_ensurer`
* context::current::scoped_override_t` now has a ctor which accepts. `primary_context_t&&`'s - to hold on to their PC reference which they are about to let go of.
* Moved: `context::current::scoped_override_t` is now implemented in the multi-wrapper implementations directory; consequently
    * Moved the implementations of  `module_t::get_kernel()` and `module::create<Creator>` to the multi-wrapper directory, since they use `context::current::scoped_override_t`.
    * Added inclusion of `cuda/api/multi_wrapper_impls/module.hpp` to some example code.
* Made a device current in some examples to avoid having no current context when executing certain operations with no wrappers (e.g. memcpy with host-side addresses)
* When allocating managed or pinned-host memory, now increasing the reference of some  context by 1 (choosing the primary context of device 0 since that's the safest), and decreasing it again on destruction. That guarantees that operations involving that allocated memory will not occur with no constructed contexts.
    * Corresponding comment changes on the `allocate()` and `free()` methods for pinned-host and managed memory.
* Factored out the code in `context_t::is_primary()` to a function, `cuda::context::current::detail_::is_primary`, which can now also be used via `cuda::context::current::is_primary()`.
* Kernel launch functions now ensure a launch only occurs / is enqueued within a current context (any context).
* Getting the current device now ensures its primary context is also active (which getting an arbitrary device does not do so).
* Added doxygen comment for `device::detail_::wrap()` mentioning the primary context reference behavior.
eyalroz added a commit that referenced this issue Apr 16, 2022
…t context, primary contexts, and ensuring their existence in various circumstanves:

* Renamed: `context::current::detail_::scoped_current_device_fallback_t` -> `scoped_existence_ensurer_t` `context::current::detail_::scoped_context_existence_ensurer`
* context::current::scoped_override_t` now has a ctor which accepts. `primary_context_t&&`'s - to hold on to their PC reference which they are about to let go of.
* Moved: `context::current::scoped_override_t` is now implemented in the multi-wrapper implementations directory; consequently
    * Moved the implementations of  `module_t::get_kernel()` and `module::create<Creator>` to the multi-wrapper directory, since they use `context::current::scoped_override_t`.
    * Added inclusion of `cuda/api/multi_wrapper_impls/module.hpp` to some example code.
* Made a device current in some examples to avoid having no current context when executing certain operations with no wrappers (e.g. memcpy with host-side addresses)
* When allocating managed or pinned-host memory, now increasing the reference of some  context by 1 (choosing the primary context of device 0 since that's the safest), and decreasing it again on destruction. That guarantees that operations involving that allocated memory will not occur with no constructed contexts.
    * Corresponding comment changes on the `allocate()` and `free()` methods for pinned-host and managed memory.
* Factored out the code in `context_t::is_primary()` to a function, `cuda::context::current::detail_::is_primary`, which can now also be used via `cuda::context::current::is_primary()`.
* Kernel launch functions now ensure a launch only occurs / is enqueued within a current context (any context).
* Getting the current device now ensures its primary context is also active (which getting an arbitrary device does not do so).
* Added doxygen comment for `device::detail_::wrap()` mentioning the primary context reference behavior.
eyalroz added a commit that referenced this issue Apr 21, 2022
…t context, primary contexts, and ensuring their existence in various circumstanves:

* Renamed: `context::current::detail_::scoped_current_device_fallback_t` -> `scoped_existence_ensurer_t` `context::current::detail_::scoped_context_existence_ensurer`
* context::current::scoped_override_t` now has a ctor which accepts. `primary_context_t&&`'s - to hold on to their PC reference which they are about to let go of.
* Moved: `context::current::scoped_override_t` is now implemented in the multi-wrapper implementations directory; consequently
    * Moved the implementations of  `module_t::get_kernel()` and `module::create<Creator>` to the multi-wrapper directory, since they use `context::current::scoped_override_t`.
    * Added inclusion of `cuda/api/multi_wrapper_impls/module.hpp` to some example code.
* Made a device current in some examples to avoid having no current context when executing certain operations with no wrappers (e.g. memcpy with host-side addresses)
* When allocating managed or pinned-host memory, now increasing the reference of some  context by 1 (choosing the primary context of device 0 since that's the safest), and decreasing it again on destruction. That guarantees that operations involving that allocated memory will not occur with no constructed contexts.
    * Corresponding comment changes on the `allocate()` and `free()` methods for pinned-host and managed memory.
* Factored out the code in `context_t::is_primary()` to a function, `cuda::context::current::detail_::is_primary`, which can now also be used via `cuda::context::current::is_primary()`.
* Kernel launch functions now ensure a launch only occurs / is enqueued within a current context (any context).
* Getting the current device now ensures its primary context is also active (which getting an arbitrary device does not do so).
* Added doxygen comment for `device::detail_::wrap()` mentioning the primary context reference behavior.
@eyalroz eyalroz closed this as completed May 16, 2022
eyalroz added a commit that referenced this issue Jun 20, 2022
…t context, primary contexts, and ensuring their existence in various circumstanves:

* Renamed: `context::current::detail_::scoped_current_device_fallback_t` -> `scoped_existence_ensurer_t` `context::current::detail_::scoped_context_existence_ensurer`
* context::current::scoped_override_t` now has a ctor which accepts. `primary_context_t&&`'s - to hold on to their PC reference which they are about to let go of.
* Moved: `context::current::scoped_override_t` is now implemented in the multi-wrapper implementations directory; consequently
    * Moved the implementations of  `module_t::get_kernel()` and `module::create<Creator>` to the multi-wrapper directory, since they use `context::current::scoped_override_t`.
    * Added inclusion of `cuda/api/multi_wrapper_impls/module.hpp` to some example code.
* Made a device current in some examples to avoid having no current context when executing certain operations with no wrappers (e.g. memcpy with host-side addresses)
* When allocating managed or pinned-host memory, now increasing the reference of some  context by 1 (choosing the primary context of device 0 since that's the safest), and decreasing it again on destruction. That guarantees that operations involving that allocated memory will not occur with no constructed contexts.
    * Corresponding comment changes on the `allocate()` and `free()` methods for pinned-host and managed memory.
* Factored out the code in `context_t::is_primary()` to a function, `cuda::context::current::detail_::is_primary`, which can now also be used via `cuda::context::current::is_primary()`.
* Kernel launch functions now ensure a launch only occurs / is enqueued within a current context (any context).
* Getting the current device now ensures its primary context is also active (which getting an arbitrary device does not do so).
* Added doxygen comment for `device::detail_::wrap()` mentioning the primary context reference behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant