Skip to content
Snippets Groups Projects
Unverified Commit d2113a27 authored by Chris Truter's avatar Chris Truter Committed by GitHub
Browse files

Fix potential deadlock when loading feature flags

Fixes https://github.com/metabase/metabase/issues/38009

### Description

This makes the feature flag loading code more robust by making the function re-entrant. To understand how this function could deadlock before, see the description in https://github.com/metabase/metabase/pull/38027.

It works by ensuring that the call to initialize the nonce comes from the same thread that initially locked on fetching the tokens.

There is a subtle behavior change here - was allow for a 10s timeout on each call individually now. Since it is unlikely that both endpoints will timeout consecutively, and the combined wait time is still fairly short, I think this is OK.

### How to verify

You can use the same steps as https://github.com/metabase/metabase/pull/38027, either of these PRs are sufficient to fix the bug.
parent 926b04a1
No related branches found
No related tags found
No related merge requests found
......@@ -96,15 +96,28 @@
[:trial {:optional true} :boolean]
[:valid-thru {:optional true} ms/NonBlankString]]) ; ISO 8601 timestamp
(defn- fetch-token-and-parse-body
[token base-url]
(defn- fetch-token-and-parse-body*
[token base-url site-uuid]
(some-> (token-status-url token base-url)
(http/get {:query-params {:users (cached-active-users-count)
:site-uuid (setting/get :site-uuid-for-premium-features-token-checks)
:site-uuid site-uuid
:mb-version (:tag config/mb-version-info)}})
:body
(json/parse-string keyword)))
(defn- fetch-token-and-parse-body
[token base-url site-uuid]
(let [fut (future (fetch-token-and-parse-body* token base-url site-uuid))
result (deref fut fetch-token-status-timeout-ms ::timed-out)]
(if (not= result ::timed-out)
result
(do
(future-cancel fut)
{:valid false
:status (tru "Unable to validate token")
:error-details (tru "Token validation timed out.")}))))
(mu/defn ^:private fetch-token-status* :- TokenStatus
"Fetch info about the validity of `token` from the MetaStore."
[token :- :string]
......@@ -117,32 +130,26 @@
{:valid false
:status "invalid"
:error-details (trs "Token should be 64 hexadecimal characters.")})
(let [fut (future
(try (fetch-token-and-parse-body token token-check-url)
(catch Exception e1
(log/error e1 (trs "Error fetching token status from {0}:" token-check-url))
;; Try the fallback URL, which was the default URL prior to 45.2
(try (fetch-token-and-parse-body token store-url)
;; if there was an error fetching the token from both the normal and fallback URLs, log the
;; first error and return a generic message about the token being invalid. This message
;; will get displayed in the Settings page in the admin panel so we do not want something
;; complicated
(catch Exception e2
(log/error e2 (trs "Error fetching token status from {0}:" store-url))
(let [body (u/ignore-exceptions (some-> (ex-data e1) :body (json/parse-string keyword)))]
(or
body
{:valid false
:status (tru "Unable to validate token")
:error-details (.getMessage e1)})))))))
result (deref fut fetch-token-status-timeout-ms ::timed-out)]
(if (= result ::timed-out)
(do
(future-cancel fut)
{:valid false
:status (tru "Unable to validate token")
:error-details (tru "Token validation timed out.")})
result))))
(let [site-uuid (setting/get :site-uuid-for-premium-features-token-checks)]
(try (fetch-token-and-parse-body token token-check-url site-uuid)
(catch Exception e1
;; Unwrap exception from inside the future
(let [e1 (ex-cause e1)]
(log/error e1 (trs "Error fetching token status from {0}:" token-check-url))
;; Try the fallback URL, which was the default URL prior to 45.2
(try (fetch-token-and-parse-body token store-url site-uuid)
;; if there was an error fetching the token from both the normal and fallback URLs, log the
;; first error and return a generic message about the token being invalid. This message
;; will get displayed in the Settings page in the admin panel so we do not want something
;; complicated
(catch Exception e2
(log/error (ex-cause e2) (trs "Error fetching token status from {0}:" store-url))
(let [body (u/ignore-exceptions (some-> (ex-data e1) :body (json/parse-string keyword)))]
(or
body
{:valid false
:status (tru "Unable to validate token")
:error-details (.getMessage e1)}))))))))))
(def ^{:arglists '([token])} fetch-token-status
"TTL-memoized version of `fetch-token-status*`. Caches API responses for 5 minutes. This is important to avoid making
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment