Refactoring Bytes to check double free and code cleanup#22251
Refactoring Bytes to check double free and code cleanup#22251cpegeric wants to merge 10 commits intomatrixorigin:mainfrom
Conversation
fengttt
left a comment
fengttt
left a comment
There was a problem hiding this comment.
Please fix the panic. Also check/verify that we always have set refcount to 1 when creating a cached "bytes".
| idx := int(cacheData.Index) | ||
| if cacheData.Hit { | ||
| vector.Entries[idx].done = true | ||
| vector.Entries[idx].CachedData = &Bytes{bytes: cacheData.Data} |
There was a problem hiding this comment.
@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.
旧的代码不带引用计数，所以就只是对 []byte 的包装。新的代码改成了引用计数，只要使用方的计数正确，就不会有问题。如果真的有问题，可以另外增加一个 GoBytes 类型，实现 fscache.Data 接口，但不实现引用计数机制。
There was a problem hiding this comment.
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.
| } | ||
| bytes._refs.Store(1) | ||
| bytes.refs = &bytes._refs | ||
| bytes.refs.Store(1) |
There was a problem hiding this comment.
Minor, you can do a NewBytes here, so that we can restrict all the Store(1) to one single place.
|
Very true. In math and CS, intuition often comes from familiarity over time, not instant redline custom co understanding. |
5 participants
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