Skip to content

Use Pybind11's CMake code#667

Merged
sloretz merged 3 commits intomasterfrom
add_pybind11_dependency
Feb 12, 2021
Merged

Use Pybind11's CMake code#667
sloretz merged 3 commits intomasterfrom
add_pybind11_dependency

Conversation

@sloretz
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

cottsay commented Jan 27, 2021 •
edited
Loading

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

Details 
* 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

clalancette commented Feb 13, 2021

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