Skip to content

Return SessionPool sessions after JSON errors - #151

Open
chenshi5012 wants to merge 1 commit into
vesoft-inc:masterfrom
chenshi5012:fix/json-exception-returns-session
Open

chenshi5012 wants to merge 1 commit into
vesoft-inc:masterfrom
chenshi5012:fix/json-exception-returns-session

Conversation

@chenshi5012

@chenshi5012 chenshi5012 commented Oct 15, 2026 •

Copy link
Copy Markdown

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number: #149

Description:

When a low-level JSON request fails, Connection can return an empty string. SessionPool::executeJsonWithParameter( ) then calls folly::parseJson() unconditionally. If parsing throws, the normal giveBack() path is skipped. Although the local Session may return its underlying connection to ConnectionPool during destruction, the Session wrapper is not returned to SessionPool::idleSessions_, permanently consuming a pool slot.

With a small pool, or after repeated failures, subsequent requests can fail with No idle session.

How do you solve it?

Return the acquired Session to SessionPool in the exception path before rethrowing the original exception. The normal success path continues to return the session as before.

This ensures that a failed JSON request or a JSON parsing exception does not permanently reduce the number of available session slots.

Special notes for your reviewer, ex. impact of this fix, design document, etc:

  • Added the regression test SessionPoolTest.JsonParseFailureReturnsSessionToPool. It closes the underlying connection to deterministically produce an empty JSON response, verifies that parsing throws, and checks that the session slot remains available afterward.
  • The focused regression test passed.
  • The full session_pool_test CTest target passed.

Fixes #149

@CLAassistant

CLAassistant commented Oct 15, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@chenshi5012
chenshi5012 force-pushed the fix/json-exception-returns-session branch from ba6dc1d to 8713bbc Compare October 15, 2026 01:52

This branch has not been deployed

No deployments
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.

[p1-high]SessionPool::executeJson loses a session slot when parsing a failed/empty JSON response

2 participants