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

unique_ptr: Fix ownership issues for failed overloads (RobotLocomotion/drake#8160) #11

Merged

Conversation

EricCousineau-TRI
Copy link
Collaborator

@EricCousineau-TRI EricCousineau-TRI commented Feb 23, 2018

This change is Reviewable

@EricCousineau-TRI EricCousineau-TRI changed the title unique_ptr: Fix ownership issues for failed overloads (RobotLocomotion/drake#8160) [WIP] unique_ptr: Fix ownership issues for failed overloads (RobotLocomotion/drake#8160) Feb 23, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@EricCousineau-TRI
Copy link
Collaborator Author

EricCousineau-TRI commented Feb 23, 2018

@basicNew @apojomovsky I've submitted the fix for this, and have sent y'all an invite as collaborators for this repo (for review).

Could one of y'all do review for this PR? (just a high-level overview - looking at the changes, and ensure that this actually captures the root issue for RobotLocomotion/drake#8160)

NOTE: This fork has broken support on Windows (AppVeyor), so those tests will fail. I have deferred fixing those until it becomes important for this fork (either it'll be upstreamed, or someone wants Windows support).

@EricCousineau-TRI EricCousineau-TRI changed the title [WIP] unique_ptr: Fix ownership issues for failed overloads (RobotLocomotion/drake#8160) unique_ptr: Fix ownership issues for failed overloads (RobotLocomotion/drake#8160) Feb 23, 2018
EricCousineau-TRI added a commit to EricCousineau-TRI/drake that referenced this pull request Feb 23, 2018
@apojomovsky
Copy link

:lgtm:


Review status: 0 of 3 files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@apojomovsky
Copy link

Reviewed 2 of 2 files at r1, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@EricCousineau-TRI EricCousineau-TRI merged commit 3dcfcb0 into RobotLocomotion:drake Feb 26, 2018
EricCousineau-TRI added a commit to EricCousineau-TRI/drake that referenced this pull request Feb 26, 2018
EricCousineau-TRI added a commit to RobotLocomotion/drake that referenced this pull request Feb 26, 2018
pybind11: Incorporate RobotLocomotion/pybind11#11 (unique_ptr overload transactions)
jwnimmer-tri pushed a commit to jwnimmer-tri/pybind11 that referenced this pull request Dec 24, 2024
* Support free-threaded CPython (PEP 703)

Some additional locking is added in the free-threaded build when
`Py_GIL_DISABLED` is defined:

- Most accesses to internals are protected by a single mutex
- The registered_instances uses a striped lock to improve concurrency

Pybind11 modules can indicate support for running with the GIL disabled
by calling `set_gil_not_used()`.

* refactor: use PYBIND11_MODULE (RobotLocomotion#11)

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

* Address code review

* Suppress MSVC warning

* Changes from review

* style: pre-commit fixes

* `py::mod_gil_not_used()` suggestion.

* Update include/pybind11/pybind11.h

---------

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.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.

None yet

2 participants