Skip to content
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

Methods that continuously call WeakReference wrapper may throw an NPE #446

Open
liutaohua opened this issue Jul 19, 2021 · 7 comments · May be fixed by #4872
Open

Methods that continuously call WeakReference wrapper may throw an NPE #446

liutaohua opened this issue Jul 19, 2021 · 7 comments · May be fixed by #4872
Assignees
Labels
enhancement New feature or request good first issue Issues for first-time contributors

Comments

@liutaohua
Copy link
Contributor

All methods using WeakReference must check whether it is NULL after calling get().

image

@liutaohua liutaohua added the bug Something isn't working label Jul 19, 2021
@ruanwenjun
Copy link
Member

Yes, this does happen when the system gc occur, maybe we can use strong reference instead of weak reference.

@liutaohua
Copy link
Contributor Author

Yes, this does happen when the system gc occur, maybe we can use strong reference instead of weak reference.

I need to continue to understand the business logic in order to understand the author's intentions.

We can wait for more comments on this question.

@lrhkobe
Copy link
Contributor

lrhkobe commented Jul 19, 2021

The weak reference of ClientGroupWrapper in Session.java is designed to avoid circular reference.
Because ClientGroupWrapper contains the reference of Session, and Session contains the reference of ClientGroupWrapper.

You are right. When system GC occurs, weak references will be recycled, but if the strong reference of ClientGroupWrapper also exists, maybe it will not be a problem.

We can find strong reference in the clientGroupMap field of ClientSessionGroupMapping.java :

private ConcurrentHashMap<String, ClientGroupWrapper> clientGroupMap;

Copy link
Contributor

It has been 90 days since the last activity on this issue. Apache EventMesh values the voices of the community. Please don't hesitate to share your latest insights on this matter at any time, as the community is more than willing to engage in discussions regarding the development and optimization directions of this feature.

If you feel that your issue has been resolved, please feel free to close it. Should you have any additional information to share, you are welcome to reopen this issue.

@github-actions github-actions bot added the Stale label Apr 23, 2024
@Pil0tXia Pil0tXia added good first issue Issues for first-time contributors enhancement New feature or request and removed Stale bug Something isn't working labels Apr 24, 2024
@zzzk1
Copy link

zzzk1 commented May 3, 2024

@Pil0tXia, pls assigned this issue to me.

@Pil0tXia
Copy link
Member

Pil0tXia commented May 3, 2024

@zzzk1 OK

@pandaapo
Copy link
Contributor

pandaapo commented May 7, 2024

Community member explained why the design is like this and why there is no need to worry about the potential weak reference problem (#446 (comment)). Could anyone explain why this viewpoint is not recognized and this issue must be fixed?

社区成员解释了为什么这样设计以及为什么不需要担心弱引用的潜在问题(#446 (comment) ),谁能解释一下该观点不被认可、必须修复该issue的原因?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Issues for first-time contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants