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

Python Wheel using dynamic linking (e.g. built with --cpp_implementation not --cpp_implementation --compile_static_extension) #8291

Closed
rowillia opened this issue Feb 12, 2021 · 1 comment
Assignees

Comments

@rowillia
Copy link
Contributor

What language does this apply to?
Python
If it's a proto syntax change, is it for proto2 or proto3? N/A
If it's about generated code change, what programming language? Python

Describe the problem you are trying to solve.

Trying to use protobuf with cpp_implementation in the same python process as a library that's also using protobuf under the covers.

This seems to be a widespread issue in the tensorflow community. My issue isn't with tensorflow but the problem is identical. Tensorflow seems to have fixed this by suggesting people pin to an older version of protobuf before the wheels started shipping the native extension.

(e.g. tensorflow/tensorflow#8394,
tensorflow/tensorflow#9525 (comment)
tensorflow/tensorflow#24976,
tensorflow/tensorflow#35573,
https://github.com/tensorflow/tensorflow/blob/v2.0.0/tensorflow/contrib/makefile/rename_protobuf.sh,
tensorflow/tensorflow#16104)

Describe the solution you'd like
An option to install a wheel which relies on dynamic linking instead of static linking, allowing both libraries to reference protobuf without causing segfaults and undefined behavior.

I think this could take a few forms:

  • We could add the native extension as a an extra (e.g. users would pip install protobuf[cpp] to get the cpp extension).
  • We could ship libprotobuf.*.{dylib, so, dll} as package_data and explicitly load it at import time with cdll which would allow other libraries to also find and load them.
  • We could publish another package all together (e.g. protobufdynamic)

Describe alternatives you've considered

Additional context
OSX defaults to RTLD_GLOBAL when loading libraries -https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/dlopen.3.html

@rowillia
Copy link
Contributor Author

After further research #8346 should obviate the need for this.

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

No branches or pull requests

3 participants