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
Memory leak introduced in cc8ca5b6a5 #2385
Comments
You said you can reproduce this locally -- are you able to do so in a debug build that can dump allocation stacks with symbols? That might make it easier to track this down. |
Unfortunately, I cannot get a stack with symbols from my local environment, even in a debug build. However I was able to make progress on this issue : current Chromium build of protobuf forces GOOGLE_PROTOBUF_NO_STATIC_INITIALIZER define (in BUILD.gn). If I remove this define, the leak goes away. I was also able to narrow down the change causing the regression to src/google/protobuf/compiler/cpp/cpp_file.cc and/or src/google/protobuf/compiler/cpp/cpp_message.cc file(s) of cc8ca5b. |
So far, the best call stacks I can get look like this :
Where
|
By comparing the generated code, I think the actual problem is that GOOGLE_PROTOBUF_NO_STATIC_INITIALIZER define logic has been inverted between 337a028 and cc8ca5b for c++ (at least). As a matter of fact, the CdmAdapterTest.Initialize Chromium test :
So the leak wouldn't be a leak per say, but just a detection of statically initialized objects that are not freed (as we could expect). I'll try to work on a fix on monday |
Behavior of define GOOGLE_PROTOBUF_NO_STATIC_INITIALIZER has been altered between 337a028 and cc8ca5b for C++. See github issue protocolbuffers#2385 for further details.
It is very likely that cc8ca5b introduced a memory leak.
The linux_chromium_asan_rel_ng Chromium try bot reports memory leaks with this commit (and next ones), whereas no leak is reported with its parent (and previous ones).
Example of leaking unit test : CdmAdapterTest.Initialize
See Chromium CL comments for further information
The text was updated successfully, but these errors were encountered: