Skip to content

Fix detection of gevent threading.local monkey-patch#447

Merged
untitaker merged 3 commits into
getsentry:masterfrom
bluetech:fix-gevent-threading-detection
Aug 6, 2019
Merged

Fix detection of gevent threading.local monkey-patch#447
untitaker merged 3 commits into
getsentry:masterfrom
bluetech:fix-gevent-threading-detection

Conversation

@bluetech

@bluetech bluetech commented Aug 4, 2019

Copy link
Copy Markdown
Contributor

The _ in _threading seems like a mistake and always returns False.
Removing it leads to the detection working properly:

Python 3.7.4 (default, Jul 16 2019, 07:12:58)
[GCC 9.1.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import gevent.monkey
>>> gevent.monkey.is_object_patched('_threading', 'local')
False
>>> gevent.monkey.is_object_patched('threading', 'local')
False
>>> gevent.monkey.patch_all()
True
>>> gevent.monkey.is_object_patched('_threading', 'local')
False
>>> gevent.monkey.is_object_patched('threading', 'local')
True

The `_` in `_threading` seems like a mistake and always returns False.
Removing it leads to the detection working properly:

```py
Python 3.7.4 (default, Jul 16 2019, 07:12:58)
[GCC 9.1.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import gevent.monkey
>>> gevent.monkey.is_object_patched('_threading', 'local')
False
>>> gevent.monkey.is_object_patched('threading', 'local')
False
>>> gevent.monkey.patch_all()
True
>>> gevent.monkey.is_object_patched('_threading', 'local')
False
>>> gevent.monkey.is_object_patched('threading', 'local')
True
```
@bluetech

bluetech commented Aug 4, 2019

Copy link
Copy Markdown
Contributor Author

We are using gevent and are seeing some data "leaking" between events. I hope this will fix the problem, but I did not verify it.

@untitaker

Copy link
Copy Markdown
Member

Thanks!

@untitaker
untitaker merged commit a50b651 into getsentry:master Aug 6, 2019


def test_gevent_is_patched():
gevent.monkey.patch_all()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wonder, does this not affect all of the subsequent tests in the test run?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We run each test in its own subprocess because the SDK itself is already so heavy on monkeypatching.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ha, nice.

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

Successfully merging this pull request may close these issues.

2 participants