Skip to content

Conversation

@Tsukilc
Copy link

@Tsukilc Tsukilc commented May 6, 2025

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

Use scheduled tasks to detect whether futures have timeout information before sending messages. If there is, clear it to avoid memory leaks

Ⅱ. Does this pull request fix one issue?

fixes #5283

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

mergeSendExecutorService.shutdown();
}

futures.clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it necessary to clean up these maps? When a RemotingClient executes the destroy method, this instance should no longer be used. In theory, it should be garbage collected, or the application is shutting down.

Copy link
Author

Choose a reason for hiding this comment

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

Yes. manually clearing the maps is indeed unnecessary.I have removed it.

Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

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

I believe the following PR can solve this issue:
#7095

@Tsukilc Tsukilc closed this May 6, 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.

ClientOnResponseProcessor#mergeMsgMap&futures is a risk of memory leaks

2 participants