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

Safe handler dictionary iteration #326

Closed
colebaileygit opened this issue May 1, 2024 · 5 comments · Fixed by #328
Closed

Safe handler dictionary iteration #326

colebaileygit opened this issue May 1, 2024 · 5 comments · Fixed by #328
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@colebaileygit
Copy link

I got this stack trace while running tests. I imagine it is a very rare race condition:

Can we make the handler data object safe for multi-threading?

tests/e2e/inprocess/grpc/test_inprocess_grpc_reconnect.py::test_provider_unavailable
  /Users/c.bailey/Library/Application Support/hatch/env/virtual/openfeature-provider-flagd/2sJ2q5Uj/openfeature-provider-flagd/lib/python3.9/site-packages/_pytest/threadexception.py:77: PytestUnhandledThreadExceptionWarning: Exception in thread Thread-3
  
  Traceback (most recent call last):
    File "/Users/c.bailey/.pyenv/versions/3.9.13/lib/python3.9/threading.py", line 980, in _bootstrap_inner
      self.run()
    File "/Users/c.bailey/.pyenv/versions/3.9.13/lib/python3.9/threading.py", line 917, in run
      self._target(*self._args, **self._kwargs)
    File "/Users/c.bailey/Source/python-sdk-contrib/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/connector/grpc_watcher.py", line 95, in sync_flags
      self.provider.emit_provider_error(
    File "/Users/c.bailey/Library/Application Support/hatch/env/virtual/openfeature-provider-flagd/2sJ2q5Uj/openfeature-provider-flagd/lib/python3.9/site-packages/openfeature/provider/provider.py", line 81, in emit_provider_error
      self.emit(ProviderEvent.PROVIDER_ERROR, details)
    File "/Users/c.bailey/Library/Application Support/hatch/env/virtual/openfeature-provider-flagd/2sJ2q5Uj/openfeature-provider-flagd/lib/python3.9/site-packages/openfeature/provider/provider.py", line 87, in emit
      run_handlers_for_provider(self, event, details)
    File "/Users/c.bailey/Library/Application Support/hatch/env/virtual/openfeature-provider-flagd/2sJ2q5Uj/openfeature-provider-flagd/lib/python3.9/site-packages/openfeature/_event_support.py", line 75, in run_handlers_for_provider
      for client in _client_handlers:
  RuntimeError: dictionary changed size during iteration
  
    warnings.warn(pytest.PytestUnhandledThreadExceptionWarning(msg))
@beeme1mr
Copy link
Member

beeme1mr commented May 1, 2024

Relates to #96.

@colebaileygit would this be something you would be willing to help with? FYI @federicobond @gruebel @toddbaert

@beeme1mr beeme1mr added bug Something isn't working help wanted Extra attention is needed labels May 1, 2024
@federicobond
Copy link
Member

Thanks for reporting @colebaileygit, it's nice to have an existence proof for the thread-safety issues we knew were there.

federicobond added a commit to federicobond/python-sdk that referenced this issue May 2, 2024
The _client_handlers dictionary allowed modifications during iteration
without proper concurrency control. I added some reentrant locks to manage
concurrent access to the _global_handlers and _client_handlers data
structures.

See open-feature#326

Signed-off-by: Federico Bond <federicobond@gmail.com>
@federicobond
Copy link
Member

I wrote a test that can reproduce the issue 100% of the time on my computer and implemented some locks to prevent concurrent access to the event handler dictionaries. See #329

@federicobond
Copy link
Member

There is still quite a lot of thread-safety work to be done with other methods, particularly in the openfeature.api module.

federicobond added a commit that referenced this issue May 2, 2024
The _client_handlers dictionary allowed modifications during iteration
without proper concurrency control. I added some reentrant locks to manage
concurrent access to the _global_handlers and _client_handlers data
structures.

See #326

Signed-off-by: Federico Bond <federicobond@gmail.com>
@federicobond
Copy link
Member

I think we can mark this particular issue as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants