Repository navigation
Conversation
ResourcePool, ObjectPool, their LocalPool and SocketPool are declared with BAIDU_CACHELINE_ALIGNMENT and created with plain new. C++17 uses aligned operator new for them, but C++14 builds (the CMake and Make default with protobuf < 22) do not, and glibc returns only 16-byte aligned storage. With Clang 14 this corrupts sockets created during a client connect and crashes in Socket::ResetFileDescriptor. Pass -faligned-new when the compiler accepts it, and add a test that fails when aligned new is not in effect.
3 of 4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
ResourcePool,ObjectPool, theirLocalPoolandSocketPoolare declared withBAIDU_CACHELINE_ALIGNMENTand created with plainnew. C++17 uses the alignedoperator newfor them. C++14 builds don't, and glibc then returns only 16-byte aligned storage. CMake andconfig_brpc.shboth pick C++14 when protobuf < 22.On Ubuntu 22.04 with Clang 14, this crashes client connects in
Socket::ResetFileDescriptor(_transportis null). It reproduces 3/3 with the FlatBuffers protocol tests in #3559. Rebuilding onlysocket.cppwith-faligned-newor-std=gnu++17fixes it, and-std=gnu++17 -fno-aligned-newbrings the crash back. GCC 11 in C++14 mode misaligns these objects the same way, but it did not crash in those tests.What is changed and the side effects?
config_brpc.shpass-faligned-newwhen the compiler accepts it. Bazel already builds with C++17 and is unchanged.ResourcePoolTest.plain_new_honors_cacheline_alignmentfails when aligned new is not in effect.operator new: libstdc++ from GCC 7, or libc++.Tests
Ubuntu 22.04 Clang 14: the new test passes, and fails with
-DBRPC_HAS_ALIGNED_NEW=OFF.config_brpc.shwith Clang 14 and GCC 11 produces-std=c++14 -faligned-new. macOS Apple clang 15 detects the flag.