Skip to content

Fix invalid AuthMech crashing with Recursive update / StackOverflowError#1558

Open
sreekanth-db wants to merge 1 commit into
databricks:mainfrom
sreekanth-db:fix/invalid-authmech-fail-fast
Open

Fix invalid AuthMech crashing with Recursive update / StackOverflowError#1558
sreekanth-db wants to merge 1 commit into
databricks:mainfrom
sreekanth-db:fix/invalid-authmech-fail-fast

Conversation

@sreekanth-db

Copy link
Copy Markdown
Collaborator

Summary

  • Connecting with an unsupported AuthMech (e.g. AuthMech=99) intermittently failed with an internal IllegalStateException: Recursive update or a StackOverflowError on both the SEA and Thrift paths, instead of a clean validation error.
  • Root cause: AuthMech.parseAuthMech throws during ClientConfigurator construction, which runs inside DatabricksClientConfiguratorManager.getConfigurator's ConcurrentHashMap.computeIfAbsent. The exception's constructor emits a telemetry failure log, whose feature-flag refresh re-enters getConfigurator for the same connectionUuid → a reentrant computeIfAbsentRecursive update (or, depending on timing, StackOverflowError).
  • Fix: validate AuthMech at connect time in DatabricksConnectionContext.parse() (via ValidationUtil.validateInputProperties → new validateAuthMech), before the configurator machinery runs. Unsupported values are now rejected deterministically with a DatabricksValidationException (SQLState=INPUT_VALIDATION_ERROR).
  • A pure, side-effect-free AuthMech.fromValue(int) lookup is introduced as the single source of truth for supported values (3=PAT, 11=OAuth); parseAuthMech delegates to it, so adding a new AuthMech is a one-line change. parseAuthMech's behavior, exception type, and messages are unchanged.

Test plan

  • New unit tests ValidationUtilTest#testValidateAuthMech_*: supported values (3, 11, omitted) pass; unsupported/non-integer/empty throw INPUT_VALIDATION_ERROR.
  • New unit test DatabricksConnectionContextTest#parse_rejectsUnsupportedAuthMech_withInputValidationError.
  • Existing AuthFlowParsing and full ValidationUtilTest (47) / DatabricksConnectionContextTest (145) suites pass.
  • Manual repro against a live warehouse: AuthMech=99 went from IllegalStateException: Recursive update (5/5 attempts on both SEA and Thrift) to a clean DatabricksValidationException / INPUT_VALIDATION_ERROR (5/5 on both transports).

Connecting with an unsupported AuthMech (e.g. AuthMech=99) intermittently
crashed with an internal IllegalStateException: Recursive update or a
StackOverflowError on both SEA and Thrift, instead of a clean validation error.

The value was detected deep inside ClientConfigurator construction, which runs
inside getConfigurator's ConcurrentHashMap.computeIfAbsent; the failure fired a
telemetry log whose feature-flag refresh re-entered getConfigurator for the same
connectionUuid, causing a reentrant computeIfAbsent.

Validate AuthMech at connect time (DatabricksConnectionContext.parse ->
ValidationUtil.validateInputProperties) before the configurator machinery runs,
rejecting unsupported values deterministically with a DatabricksValidationException
(SQLState=INPUT_VALIDATION_ERROR). A pure AuthMech.fromValue lookup is the single
source of truth for supported values.

Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
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