Skip to content

fix(logservice): avoid nil client deref in truncate when client creation fails#24862

Open
ck89119 wants to merge 1 commit into
matrixorigin:3.0-devfrom
ck89119:issue-24861-3.0-dev
Open

fix(logservice): avoid nil client deref in truncate when client creation fails#24862
ck89119 wants to merge 1 commit into
matrixorigin:3.0-devfrom
ck89119:issue-24861-3.0-dev

Conversation

@ck89119
Copy link
Copy Markdown
Contributor

@ck89119 ck89119 commented Jun 5, 2026

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #24861

What this PR does / why we need it:

Handle any error returned by GetOnFly in truncateFromRemote before deferring client.Putback(), so TN does not panic when logservice client creation fails. Add a regression test that covers the nil-client error path.

…ion fails

GetOnFly may return (nil, err) when the client factory fails to create a client
(e.g. logservice.NewClient times out). truncateFromRemote only handled
ErrClientPoolClosed, so on any other error it ran defer client.Putback() on a
nil *wrappedClient and panicked (SIGSEGV), crashing the TN. Return on any error
from GetOnFly, matching the sibling Get() path. Adds a regression test.

issue matrixorigin#24861

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working size/S Denotes a PR that changes [10,99] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants