Skip to content

Fix the crash caused by interface null value judgment error in golang. #2245

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 4, 2025

Conversation

linrunqi08
Copy link
Collaborator

@linrunqi08 linrunqi08 commented Jun 3, 2025

问题现象:
非容器场景,但是配置了容器的采集配置,golang模块会持续crash在docker client处

问题原因:
docker client为Interface类型,Go 语言规范规定:

  • 接口变量仅当类型和值都为 nil 时才等于 nil
  • 当接口持有具体类型的 nil 值时,接口本身不是 nil

修复方案:
initClient失败的时候,强制dc.client = nil,这样保证类型和值都为nil

@linrunqi08 linrunqi08 changed the title Fix the crash caused by Interface null value judgment error in golang. Fix the crash caused by interface null value judgment error in golang. Jun 3, 2025
@yyuuttaaoo yyuuttaaoo requested a review from Copilot June 4, 2025 01:07
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR ensures the Docker client interface is fully reset on initialization errors to prevent stale non-nil interface values, and adds a test to verify error behavior across repeated initialization attempts.

  • Explicitly set dc.client = nil when CreateDockerClient fails
  • Added TestInitClientMutiTime to assert repeated initClient calls always error and leave client nil

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/helper/containercenter/container_center.go Reset dc.client to nil on error in initClient
pkg/helper/containercenter/container_center_test.go New TestInitClientMutiTime to validate repeated error behavior
Comments suppressed due to low confidence (2)

pkg/helper/containercenter/container_center_test.go:580

  • [nitpick] Typo in test name: 'TestInitClientMutiTime' should be 'TestInitClientMultiTime' or 'TestInitClientMultipleTimes' for clarity and consistency.
func TestInitClientMutiTime(t *testing.T) {

pkg/helper/containercenter/container_center_test.go:581

  • Missing coverage for the successful initialization path: consider adding a test that simulates a successful CreateDockerClient and verifies that initClient() sets client to a non-nil value and is idempotent on subsequent calls.
    containerCenterInstance = &ContainerCenter{}

@yyuuttaaoo
Copy link
Collaborator

这么看,通过依赖dc.client == nil来判断是否初始化本身可能就未必是Go语言推荐的做法,看起来是比较容易出错的

@linrunqi08 linrunqi08 merged commit bccf9a7 into main Jun 4, 2025
27 checks passed
linrunqi08 added a commit that referenced this pull request Jun 4, 2025
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.

3 participants