Skip to content

Commit 339a96e

Browse files
authored
Fix construction order of ServiceImpl and RequestSourceServiceImpl. (envoyproxy#877)
Also re-enable `text_gcc`. Fixes envoyproxy#872. **Details**: - `ServiceImpl` creates `Envoy::Logger::Context` which requires `Envoy::Thread::MutexBasicLockable`. - The `Envoy::Thread::MutexBasicLockable` was constructed after the `Envoy::Logger::Context`, so `Envoy::Logger::Context` became invalid during destruction of `ServiceImpl`. - this PR ensures that `Envoy::Thread::MutexBasicLockable` lives longer than `Envoy::Logger::Context`. - Fixing the same problem in the construction of `RequestSourceServiceImpl`. **Reason for `test_gcc` failure:** - envoyproxy/envoy#21884 triggered this bug, because it added a logging call initiated indirectly from the destructor of `Envoy::ProcessWide`. - This logging call happened during the destruction of `ServiceImpl` when the `Envoy::Thread::MutexBasicLockable` instance was already destroyed. - As a result the PURE virtual methods on its parent object `Envoy::Thread::BasicLockable` were called. - `test_gcc` reported it because it places a special function meant to detect this behavior in the `vtbl` of unimplemented pure virtual methods. Signed-off-by: Jakub Sobon <[email protected]>
1 parent 844d4af commit 339a96e

File tree

2 files changed

+20
-2
lines changed

2 files changed

+20
-2
lines changed

.azure-pipelines/pipelines.yml

+18
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,23 @@ stages:
6161
parameters:
6262
ciTarget: $(CI_TARGET)
6363

64+
- stage: test_gcc
65+
dependsOn: ["check"]
66+
pool: "x64-large"
67+
jobs:
68+
- job: test_gcc
69+
displayName: "do_ci.sh"
70+
strategy:
71+
maxParallel: 1
72+
matrix:
73+
test_gcc:
74+
CI_TARGET: "test_gcc"
75+
timeoutInMinutes: 120
76+
steps:
77+
- template: bazel.yml
78+
parameters:
79+
ciTarget: $(CI_TARGET)
80+
6481
- stage: sanitizers
6582
dependsOn: ["test"]
6683
pool: "x64-large"
@@ -102,6 +119,7 @@ stages:
102119
- stage: release
103120
dependsOn:
104121
- "clang_tidy"
122+
- "test_gcc"
105123
- "sanitizers"
106124
- "coverage"
107125
condition: eq(variables['PostSubmit'], true)

source/client/service_impl.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ class ServiceImpl final : public nighthawk::client::NighthawkService::Service,
5555
void writeResponse(const nighthawk::client::ExecutionResponse& response);
5656
grpc::Status finishGrpcStream(const bool success, absl::string_view description = "");
5757

58+
Envoy::Thread::MutexBasicLockable log_lock_;
5859
std::unique_ptr<Envoy::Logger::Context> logging_context_;
5960
std::shared_ptr<Envoy::ProcessWide> process_wide_;
6061
Envoy::Event::RealTimeSystem time_system_; // NO_CHECK_FORMAT(real_time)
61-
Envoy::Thread::MutexBasicLockable log_lock_;
6262
grpc::ServerReaderWriter<nighthawk::client::ExecutionResponse,
6363
nighthawk::client::ExecutionRequest>* stream_;
6464
std::future<void> future_;
@@ -95,8 +95,8 @@ class RequestSourceServiceImpl final
9595
nighthawk::request_source::RequestStreamRequest>* stream) override;
9696

9797
private:
98-
std::unique_ptr<Envoy::Logger::Context> logging_context_;
9998
Envoy::Thread::MutexBasicLockable log_lock_;
99+
std::unique_ptr<Envoy::Logger::Context> logging_context_;
100100
RequestSourcePtr createStaticEmptyRequestSource(const uint32_t amount);
101101
};
102102

0 commit comments

Comments
 (0)