-
Notifications
You must be signed in to change notification settings - Fork 285
Refactoring Bytes to check double free and code cleanup #22251
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the panic. Also check/verify that we always have set refcount to 1 when creating a cached "bytes".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@reusee can you please check this line? This is significant. I will be surprised if the old code works, and the new code also works without a leak. Are we fixing a real bug here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
旧的代码不带引用计数,所以就只是对 []byte 的包装。新的代码改成了引用计数,只要使用方的计数正确,就不会有问题。如果真的有问题,可以另外增加一个 GoBytes 类型,实现 fscache.Data 接口,但不实现引用计数机制。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Cannot have another type; the ref counting MUST be correct. So great, let's go with this and see if we will hit panic.
Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, you can do a NewBytes here, so that we can restrict all the Store(1) to one single place.
Uh oh!
There was an error while loading. Please reload this page.
What type of PR is this?
Which issue(s) this PR fixes:
issue #22250
What this PR does / why we need it:
Bytes to check double free and code cleanup