Skip to content

Conversation

@LisaHusband
Copy link

@LisaHusband LisaHusband commented Jun 26, 2025

Description

This PR fixes a concurrency-related bug in Flask App Builder’s default API filter handling.
About issue

🐞 Bug Summary

In the default implementation of the FAB API model, Filters instances are often stored as class-level instance variables (e.g., self._filters). Under concurrent request processing—especially in async or multi-threaded environments—this leads to shared mutable state across requests. As a result:

  • self._filters is mutated simultaneously by different request handlers;
  • Filters from one request may "bleed into" another, returning incorrect or mixed query results;
  • No explicit error is raised, making the bug hard to detect;
  • Disabling cache does not resolve the issue.

🔧 Fix Summary

To eliminate shared state pollution between concurrent requests:

  • The _handle_filters_args method is now refactored to instantiate a new Filters object per request;
  • This ensures isolated request-scoped filter logic, avoiding unsafe reuse of self._filters;
  • The original self._filters (if any) is preserved for non-query metadata logic like merge_search_filters().

💡 Design Notes

  • We avoided using .clear_filters() on the shared instance because it does not guarantee thread safety;
  • We chose per-request instantiation rather than object pooling for simplicity and correctness;
  • This change prepares FAB for safer use in async-capable Flask environments or threaded WSGI servers.

ADDITIONAL INFORMATION

  • Is CRUD MVC related.
  • Is Auth, RBAC security related.
  • Changes the security db schema.
  • Introduces new feature
  • Removes existing feature

@codecov
Copy link

codecov bot commented Jun 26, 2025

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.12%. Comparing base (c65e067) to head (f4ea422).
⚠️ Report is 48 commits behind head on master.

Files with missing lines Patch % Lines
flask_appbuilder/api/__init__.py 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2367       +/-   ##
===========================================
- Coverage   73.81%   55.12%   -18.70%     
===========================================
  Files          72       75        +3     
  Lines        8754     9350      +596     
===========================================
- Hits         6462     5154     -1308     
- Misses       2292     4196     +1904     
Flag Coverage Δ
python 55.12% <0.00%> (-18.70%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@LisaHusband LisaHusband changed the title fix bug about filter fix(filters): prevent shared Filters instance in concurrent requests Jun 26, 2025
@LisaHusband LisaHusband reopened this Jun 26, 2025
@LisaHusband LisaHusband marked this pull request as draft June 26, 2025 12:34
@LisaHusband LisaHusband marked this pull request as draft June 26, 2025 12:34
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.

1 participant