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.

Sign up for GitHub

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

Jump to bottom

Use Pybind11's CMake code #667

Merged
merged 3 commits into from
Feb 12, 2021
Merged

Use Pybind11's CMake code #667

merged 3 commits into from
Feb 12, 2021

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Jan 21, 2021

Part of #665

This uses Pybind11's CMake code to build the CPython extensions. Once this is merged, every module will be able to use Pybind11.

@sloretz sloretz self-assigned this Jan 21, 2021
This was referenced Jan 21, 2021
Closed
Merged
@sloretz
Copy link
Contributor Author

sloretz commented Jan 21, 2021

CI (build: --packages-above-and-dependencies rclpy test: --packages-above rclpy)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

This was referenced Jan 21, 2021
Merged
Merged
@cottsay
Copy link
Member

cottsay commented Jan 27, 2021 •
edited
Loading

Here is the full backtrace for the Segfault happening on macOS:

* thread #17, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001066af5ac librmw_cyclonedds_cpp.dylib`::rmw_wait(rmw_subscriptions_t *, rmw_guard_conditions_t *, rmw_services_t *, rmw_clients_t *, rmw_events_t *, rmw_wait_set_t *, const rmw_time_t *) at rmw_node.cpp:3291:81 [opt]
    frame #1: 0x00000001066af575 librmw_cyclonedds_cpp.dylib`::rmw_wait(subs=<unavailable>, gcs=0x000000010557ce50, srvs=0x000000010557ce80, cls=0x000000010557ce68, evs=0x000000010557ce98, wait_set=<unavailable>, wait_timeout=0x0000000000000000) at rmw_node.cpp:3421 [opt]
    frame #2: 0x000000010569551a librcl.dylib`rcl_wait(wait_set=<unavailable>, timeout=<unavailable>) at wait.c:609:19 [opt]
    frame #3: 0x000000010564e867 _rclpy.cpython-38-darwin.so`rclpy_wait(_unused_self=<unavailable>, args=<unavailable>) at _rclpy.c:3374:9 [opt]
    frame #4: 0x00000001000e966a Python`cfunction_call_varargs + 319
    frame #5: 0x00000001000e90c5 Python`_PyObject_MakeTpCall + 274
    frame #6: 0x000000010018889f Python`call_function + 804
    frame #7: 0x00000001001823f5 Python`_PyEval_EvalFrameDefault + 13137
    frame #8: 0x00000001000f6b13 Python`gen_send_ex + 244
    frame #9: 0x000000010017d401 Python`builtin_next + 89
    frame #10: 0x00000001001178a1 Python`cfunction_vectorcall_FASTCALL + 169
    frame #11: 0x00000001001886d5 Python`call_function + 346
    frame #12: 0x000000010018249f Python`_PyEval_EvalFrameDefault + 13307
    frame #13: 0x0000000100189299 Python`_PyEval_EvalCodeWithName + 2107
    frame #14: 0x00000001000e9a65 Python`_PyFunction_Vectorcall + 217
    frame #15: 0x00000001000eb9a9 Python`method_vectorcall + 135
    frame #16: 0x00000001001886d5 Python`call_function + 346
    frame #17: 0x000000010018254f Python`_PyEval_EvalFrameDefault + 13483
    frame #18: 0x0000000100189299 Python`_PyEval_EvalCodeWithName + 2107
    frame #19: 0x00000001000e9a65 Python`_PyFunction_Vectorcall + 217
    frame #20: 0x00000001001886d5 Python`call_function + 346
    frame #21: 0x00000001001823d5 Python`_PyEval_EvalFrameDefault + 13105
    frame #22: 0x00000001000e990a Python`function_code_fastcall + 106
    frame #23: 0x00000001000eba22 Python`method_vectorcall + 256
    frame #24: 0x00000001000e9351 Python`PyVectorcall_Call + 108
    frame #25: 0x00000001001826e9 Python`_PyEval_EvalFrameDefault + 13893
    frame #26: 0x00000001000e990a Python`function_code_fastcall + 106
    frame #27: 0x00000001001886d5 Python`call_function + 346
    frame #28: 0x00000001001823d5 Python`_PyEval_EvalFrameDefault + 13105
    frame #29: 0x00000001000e990a Python`function_code_fastcall + 106
    frame #30: 0x00000001001886d5 Python`call_function + 346
    frame #31: 0x00000001001823d5 Python`_PyEval_EvalFrameDefault + 13105
    frame #32: 0x00000001000e990a Python`function_code_fastcall + 106
    frame #33: 0x00000001000eba22 Python`method_vectorcall + 256
    frame #34: 0x00000001000e9351 Python`PyVectorcall_Call + 108
    frame #35: 0x00000001001fe8ca Python`t_bootstrap + 74
    frame #36: 0x00000001001c0020 Python`pythread_wrapper + 25
    frame #37: 0x00007fff6d57b2eb libsystem_pthread.dylib`_pthread_body + 126
    frame #38: 0x00007fff6d57e249 libsystem_pthread.dylib`_pthread_start + 66
    frame #39: 0x00007fff6d57a40d libsystem_pthread.dylib`thread_start + 13

Here is the site of the fault.

The test passes when RMW_IMPLEMENTATION=rmw_fastrtps_cpp.

@cottsay
Copy link
Member

cottsay commented Jan 27, 2021

I think that the changes presented here are only making the race more likely, and are not directly causing the fault. I even had the test complete successfully once during my local testing.

I have a hunch that resolving the issue presented by #663 will make the tests pass here.

@cottsay cottsay mentioned this pull request Jan 28, 2021
Merged
@cottsay
Copy link
Member

cottsay commented Jan 28, 2021

#674 should work around the problem and unblock this PR.

The problem isn't specific to macOS, we just got lucky on the other platforms.

@sloretz sloretz force-pushed the add_pybind11_dependency branch from a11af4b to 850e84b Compare January 29, 2021 01:28
@sloretz sloretz marked this pull request as ready for review February 8, 2021 17:22
@sloretz sloretz mentioned this pull request Feb 9, 2021
Merged
@sloretz sloretz force-pushed the add_pybind11_dependency branch from 850e84b to 531d36e Compare February 11, 2021 00:53
@sloretz
Copy link
Contributor Author

sloretz commented Feb 11, 2021

Rebased now that #674 is merged. I think green CI and an approving review is all this needs.

CI (build: --packages-above-and-dependencies rclpy test: --packages-above rclpy)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

cottsay
cottsay requested changes Feb 12, 2021
View reviewed changes
Copy link
Member

@cottsay cottsay left a comment

Choose a reason for hiding this comment

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

A few minor things.

@sloretz
Copy link
Contributor Author

sloretz commented Feb 12, 2021

One more CI to check package.xml change

CI (build: --packages-above-and-dependencies rclpy test: --packages-above rclpy)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

sloretz and others added 3 commits February 12, 2021 09:06
@sloretz
Use Pybind11 CMake code
8f328df 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
build_depend only on pybind11_vendor
c2cbfeb 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz @cottsay
alphabetize build_depends
674b02c 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

Co-authored-by: Scott K Logan <logans@cottsay.net>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz force-pushed the add_pybind11_dependency branch from c04dc58 to 674b02c Compare February 12, 2021 17:06
@sloretz
Copy link
Contributor Author

sloretz commented Feb 12, 2021

Ran git rebase --signoff master. Merging since that didn't change any code.

@sloretz sloretz merged commit 0ebae0c into master Feb 12, 2021
@delete-merged-branch delete-merged-branch bot deleted the add_pybind11_dependency branch February 12, 2021 17:07
@clalancette
Copy link
Contributor

I'm not sure about this, but I'm guessing this PR caused failures on Windows debug in CI: https://ci.ros2.org/view/nightly/job/nightly_win_deb/1899/console

@azeey azeey mentioned this pull request Feb 26, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cottsay cottsay cottsay approved these changes

Assignees

@sloretz sloretz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sloretz @cottsay @clalancette