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

acquire a global lock to init the OpenSSL singleton locks #2336

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/cryptography/hazmat/bindings/openssl/binding.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from __future__ import absolute_import, division, print_function

import collections
import imp
import os
import threading
import types
Expand Down Expand Up @@ -171,3 +172,21 @@ def _lock_cb(cls, mode, n, file, line):
mode, n, file, line
)
)


# https://github.com/pyca/cryptography/issues/2299 contains extensive
# discussion about this workaround. In essence, we are acquiring a global lock
# (including across all subinterpreters if we're in a mod_wsgi environment)
# initializing some C level locks, then initializing the binding and adding the
# OS random engine. This should prevent engine addition race conditions,
# although it's still unclear what happens when a subinterpreter which
# has registered the locks is destroyed. Do the callback functions still work
# or are all the objects associated with that subinterpreter destroyed?
# imp.acquire_lock() is global even in 3.4+ while import locks have moved to
# per module granularity.
try:
Copy link
Member

Choose a reason for hiding this comment

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

When is the import lock not held here? If we replaced this with assert imp.lock_held() what happens?

Copy link
Member Author

Choose a reason for hiding this comment

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

@glyph can probably more clearly articulate this, but my understanding is that Python 3.4+ uses per module locks rather than the global import lock so while for Py < 3.4 we could just call init_static_locks and _ensure_ffi_initialized at module scope and be safe, 3.4+ would require us to explicitly acquire the global import lock.

Copy link
Member

Choose a reason for hiding this comment

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

I guess that 3.4+'s imp.acquire_lock() is still global and isn't per module as well?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, ok, so in 3.4/3.5, these refer to the real global lock.

The comment should mention this.

On Sun, Sep 13, 2015 at 4:02 PM, Donald Stufft [email protected]
wrote:

In src/cryptography/hazmat/bindings/openssl/binding.py
#2336 (comment):

@@ -139,3 +140,19 @@ def _lock_cb(cls, mode, n, file, line):
mode, n, file, line
)
)
+
+
+# #2299 contains extensive
+# discussion about this workaround. In essence, we are acquiring a global lock
+# (including across all subinterpreters if we're in a mod_wsgi environment)
+# initializing some C level locks, then initializing the binding and adding the
+# OS random engine. This should prevent engine addition race conditions,
+# although it's still unclear what happens when a subinterpreter which
+# has registered the locks is destroyed. Do the callback functions still work
+# or are all the objects associated with that subinterpreter destroyed?
+try:

I guess that 3.4+'s imp.acquire_lock() is still global and isn't per
module as well?


Reply to this email directly or view it on GitHub
https://github.com/pyca/cryptography/pull/2336/files#r39351421.

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084

imp.acquire_lock()
Binding.init_static_locks()
Binding._ensure_ffi_initialized()
finally:
imp.release_lock()