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

Fix the order in which ffi.cdef() is called for the included header files #53

Closed
wbolster opened this issue Nov 12, 2013 · 5 comments
Closed
Milestone

Comments

@wbolster
Copy link
Contributor

Currently PyNaCl obtains a list of header files using glob.glob(), then calls ffi.cdef() for each header file. This breaks horribly at runtime if the order of glob() is different from what it was at build time (which may happen with some file systems, or after restoring a backup), resulting in very cryptic errors at runtime, even though the build/install step succeeded just fine:

python -m nacl.public
Traceback (most recent call last):
  File "/usr/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/home/uws/.virtualenvs/38aee272d0994fde/lib/python2.7/site-packages/PyNaCl-0.2.3-py2.7-linux-x86_64.egg/nacl/public.py", line 17, in <module>
    import nacl.c
  File "/home/uws/.virtualenvs/38aee272d0994fde/local/lib/python2.7/site-packages/PyNaCl-0.2.3-py2.7-linux-x86_64.egg/nacl/c/__init__.py", line 16, in <module>
    from nacl.c.crypto_box import (
  File "/home/uws/.virtualenvs/38aee272d0994fde/local/lib/python2.7/site-packages/PyNaCl-0.2.3-py2.7-linux-x86_64.egg/nacl/c/crypto_box.py", line 23, in <module>
    crypto_box_SECRETKEYBYTES = lib.crypto_box_secretkeybytes()
  File "/home/uws/.virtualenvs/38aee272d0994fde/local/lib/python2.7/site-packages/PyNaCl-0.2.3-py2.7-linux-x86_64.egg/nacl/_lib/__init__.py", line 76, in __getattr__
    self._lib = self.ffi.verifier.load_library()
  File "/home/uws/.virtualenvs/38aee272d0994fde/local/lib/python2.7/site-packages/cffi-0.7.2-py2.7-linux-x86_64.egg/cffi/verifier.py", line 68, in load_library
    self.compile_module()
  File "/home/uws/.virtualenvs/38aee272d0994fde/local/lib/python2.7/site-packages/PyNaCl-0.2.3-py2.7-linux-x86_64.egg/nacl/_lib/__init__.py", line 71, in _compile_module
    raise RuntimeError("Cannot compile module during runtime")
RuntimeError: Cannot compile module during runtime

Under the hood CFFI generates file names like _cffi__xd64d2119xefb54d7c.so at compile time, and tries to load a different file at runtime. The reason for this is that the CRC32 part of the file name is based on (among other things) a concatenation of the strings passed fo ffi.cdef(), in the order it was called.

The fix is to always .cdef() the headers in the same order.

@wbolster
Copy link
Contributor Author

wbolster commented Jan 4, 2014

Is there anything I can do to help get this issue resolved?

@lvh
Copy link
Member

lvh commented Jan 4, 2014

LGTM. Unfortunate that there's no test coverage, but at least it's documented there...

@lvh lvh closed this as completed Jan 4, 2014
lvh added a commit that referenced this issue Jan 4, 2014
Call ffi.cdef() for header files in predictable order

Closes #53.
@wbolster
Copy link
Contributor Author

wbolster commented Jan 4, 2014

@lvh, thanks.

Fwiw, I think things like these are hard to test. In practice the glob() order will remain the same as long as the directory is not touched (e.g. during a test run). It's when you have a home dir on NFS, or after you have restored a backup, that the problem manifests. Situations like these are outside the scope of unit tests, imho.

@lvh
Copy link
Member

lvh commented Jan 4, 2014

Unit tests probably shouldn't touch filesystems in the first place; I'm thinking more of a fake glob.glob that returns all the same files in two different orders.

@lvh
Copy link
Member

lvh commented Jan 4, 2014

(In this case, that doesn't really mean you end up not touching the filesystem, I suppose, but you get the idea: instead of trying to coerce the filesystem to produce different orders for glob.glob within a test run, you stub it out and guarantee that it does. Another reason why FilePath is better than everything in the stdlib :))

@warner warner added this to the v0.3.0 milestone Mar 4, 2015
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants