[fault-injection] Startup should setup signal handlers before initializing VMStructs and protect VMFlag::name() call#674
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets fault-injection-related startup stability in the native HotSpot integration by reordering VMStructs initialization relative to signal-handler installation and by making VMFlag::name() safer to call when crash protection isn’t active.
Changes:
- Move
VMStructs::init()from early shared initialization into the laterVM::ready()path (afterProfiler::setupSignalHandlers()). - Add SafeAccess-backed pointer loading helpers in
vmStructs.inline.hand use them to hardenVMFlag::name(). - Minor refactor in
VMFlag::find()to avoid callingname()twice.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
ddprof-lib/src/main/cpp/vmEntry.cpp |
Changes when/where VMStructs::init() runs relative to signal handler setup. |
ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h |
Adds SafeAccess-checked accessors and safe offset loads; hardens VMFlag::name(). |
ddprof-lib/src/main/cpp/hotspot/vmStructs.h |
Moves inline implementations out of the header and adds load_at_offset() API. |
ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp |
Avoids double-evaluating f->name() during flag lookup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Benchmark Results (commit 4db801f)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125801933 Commit: ✅ Within expected boundariesNo significant runtime deltas (all within run-to-run noise) and no internal-counter outliers. Runtime details (per benchmark × JDK)
Internal counter details (ddprof)ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
|
Contributor
Reliability & Chaos Results✅ All reliability & chaos checks passed Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/126272718 |
Contributor
CI Test ResultsRun: #30099184666 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-07-24 14:17:33 UTC |
|
🔄 Datadog auto-retried 1 job - 1 passed on retry 🔗 Commit SHA: a5ec8f8 | Docs | Datadog PR Page | Give us feedback! |
Contributor
Benchmark Results (commit e11fe69)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125847358 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10331 ms (21 iters) | ✅ 10422 ms (21 iters) | ≈ +0.9% (±11.6%) | — / — |
| akka-uct | 25 | ✅ 8829 ms (24 iters) | ✅ 8922 ms (24 iters) | ≈ +1.1% (±10%) | — / — |
| finagle-chirper | 21 | ✅ 5986 ms (33 iters) | ✅ 5966 ms (33 iters) | ≈ -0.3% (±25.2%) | |
| finagle-chirper | 25 | ✅ 5443 ms (36 iters) | ✅ 5509 ms (36 iters) | ≈ +1.2% (±24.7%) | |
| fj-kmeans | 21 | ✅ 2848 ms (66 iters) | ✅ 2763 ms (68 iters) | 🟢 -3% | — / — |
| fj-kmeans | 25 | ✅ 2803 ms (66 iters) | ✅ 2756 ms (68 iters) | ≈ -1.7% (±2.7%) | — / — |
| future-genetic | 21 | ✅ 2047 ms (90 iters) | ✅ 2108 ms (88 iters) | 🔴 +3% | — / — |
| naive-bayes | 21 | ✅ 1247 ms (137 iters) | ✅ 1280 ms (134 iters) | ≈ +2.6% (±32.9%) | — / — |
| naive-bayes | 25 | ✅ 1011 ms (169 iters) | ✅ 1006 ms (170 iters) | ≈ -0.5% (±31.4%) | — / — |
| reactors | 21 | ✅ 16791 ms (15 iters) | ✅ 15720 ms (15 iters) | ≈ -6.4% (±7.1%) | — / — |
| reactors | 25 | ✅ 18540 ms (15 iters) | ✅ 18546 ms (15 iters) | ≈ +0% (±5%) | — / — |
Internal counter details (ddprof)
ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
| Benchmark | JDK | Dropped rec | Dropped jvmti | Dropped trace | Skipped WC | AGCT fail | Unwind fail |
|---|---|---|---|---|---|---|---|
| akka-uct | 21 | ✅ / ✅ | ✅ / ✅ | 2 / 1 | 2007 / 1981 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 3 / 2 | 2165 / 2323 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 2 / 3 | 8786 / 8795 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 2 / ✅ | 8043 / 8456 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 1289 / 1287 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 2 | 1269 / 1248 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 2929 / 2995 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 2 | 2853 / 2907 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 3511 / 3521 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 3 / 7 | 3409 / 3488 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 2 | 1674 / 1567 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 1894 / 1909 | ✅ / ✅ | ✅ / ✅ |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
ddprof-lib/src/main/java/com/datadoghq/profiler/LibraryLoader.java:130
- The Javadoc for resolveLibraryPath() is inaccurate: it claims the returned Path is absolute and that an IOException is thrown when the resource is missing, but libraryFromClasspath() throws IllegalStateException for a missing resource and the returned Path is only made absolute by callers (e.g., loadLibrary uses toAbsolutePath()). Please align the Javadoc with the actual behavior.
/**
* Resolves the on-disk path of the bundled native library for the current OS/arch, extracting it
* from the classpath if necessary. Package-visible so tests can obtain a real file path (e.g. to
* pass via {@code -agentpath:}) without loading the library into the calling process.
*
* @param scratchDir The working scratch dir where to store the temp library file, or {@code null}
* to use {@code java.io.tmpdir}
* @return The library absolute path
* @throws IOException if the resource is not found on the classpath
*/
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
Benchmark Results (commit 1243287)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125959918 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 25 | ✅ 8865 ms (24 iters) | ✅ 8793 ms (24 iters) | ≈ -0.8% (±9.9%) | — / — |
| finagle-chirper | 21 | ✅ 5970 ms (33 iters) | ✅ 5962 ms (33 iters) | ≈ -0.1% (±25.6%) | |
| finagle-chirper | 25 | ✅ 5477 ms (36 iters) | ✅ 5497 ms (36 iters) | ≈ +0.4% (±24.1%) | |
| fj-kmeans | 21 | ✅ 2756 ms (68 iters) | ✅ 2690 ms (70 iters) | ≈ -2.4% (±2.7%) | — / — |
| fj-kmeans | 25 | ✅ 2823 ms (66 iters) | ✅ 2836 ms (66 iters) | ≈ +0.5% (±2.6%) | — / — |
| future-genetic | 21 | ✅ 2107 ms (88 iters) | ✅ 2098 ms (88 iters) | ≈ -0.4% (±2.8%) | — / — |
| future-genetic | 25 | ✅ 2040 ms (91 iters) | ✅ 2020 ms (92 iters) | ≈ -1% (±2.7%) | — / — |
| naive-bayes | 21 | ✅ 1272 ms (134 iters) | ✅ 1262 ms (136 iters) | ≈ -0.8% (±32.4%) | — / — |
| naive-bayes | 25 | ✅ 1029 ms (166 iters) | ✅ 1015 ms (169 iters) | ≈ -1.4% (±31.8%) | — / — |
| reactors | 21 | ✅ 16928 ms (15 iters) | ✅ 15448 ms (15 iters) | 🟢 -8.7% | — / — |
Internal counter details (ddprof)
ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
| Benchmark | JDK | Dropped rec | Dropped jvmti | Dropped trace | Skipped WC | AGCT fail | Unwind fail |
|---|---|---|---|---|---|---|---|
| akka-uct | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 3 | 1982 / 2056 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 2 | 2188 / 2284 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 3 | 8672 / 8520 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 2 | 8216 / 8424 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 1274 / 1258 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 3 | 1280 / 1302 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | 3 / ✅ | 2955 / 2944 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 2 | 2955 / 2936 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 3 | 3490 / 3517 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 3 / 5 | 3480 / 3451 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
ddprof-lib/src/main/cpp/vmEntry.cpp:210
VM::_libjvmis read with__atomic_load_ninVM::libjvm()but written/read non-atomically inopenJvmLibrary(). Mixing atomic and non-atomic accesses is a data race (and the plain store provides no RELEASE semantics for the ACQUIRE load), which can break on weakly ordered CPUs (arm64) and under TSAN.
CodeCache* VM::openJvmLibrary() {
if (_libjvm != nullptr) {
return _libjvm;
}
ddprof-lib/src/main/java/com/datadoghq/profiler/LibraryLoader.java:130
- The Javadoc for
resolveLibraryPathsays it throwsIOExceptionwhen the resource is not found, but the implementation throwsIllegalStateExceptionin that case (libraryFromClasspath), which can mislead callers/tests.
* @param scratchDir The working scratch dir where to store the temp library file, or {@code null}
* to use {@code java.io.tmpdir}
* @return The library absolute path
* @throws IOException if the resource is not found on the classpath
*/
Contributor
Benchmark Results (commit 0c95622)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125975028 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10256 ms (21 iters) | ✅ 10453 ms (21 iters) | ≈ +1.9% (±11.9%) | — / — |
| akka-uct | 25 | ✅ 8880 ms (24 iters) | ✅ 8827 ms (24 iters) | ≈ -0.6% (±10%) | — / — |
| finagle-chirper | 21 | ✅ 5999 ms (33 iters) | ✅ 5955 ms (33 iters) | ≈ -0.7% (±25.4%) | |
| finagle-chirper | 25 | ✅ 5616 ms (35 iters) | ✅ 5494 ms (36 iters) | ≈ -2.2% (±24.5%) | |
| fj-kmeans | 21 | ✅ 2700 ms (70 iters) | ✅ 2720 ms (69 iters) | ≈ +0.7% (±2.8%) | — / — |
| fj-kmeans | 25 | ✅ 2843 ms (66 iters) | ✅ 2834 ms (66 iters) | ≈ -0.3% (±2.6%) | — / — |
| future-genetic | 21 | ✅ 2112 ms (88 iters) | ✅ 2064 ms (90 iters) | ≈ -2.3% (±2.6%) | — / — |
| future-genetic | 25 | ✅ 1958 ms (95 iters) | ✅ 2085 ms (89 iters) | 🔴 +6.5% | — / — |
| naive-bayes | 21 | ✅ 1265 ms (135 iters) | ✅ 1277 ms (134 iters) | ≈ +0.9% (±32.9%) | — / — |
| naive-bayes | 25 | ✅ 1011 ms (169 iters) | ✅ 1021 ms (168 iters) | ≈ +1% (±31.7%) | — / — |
| reactors | 21 | ✅ 16332 ms (15 iters) | ✅ 16232 ms (15 iters) | ≈ -0.6% (±7.1%) | — / — |
| reactors | 25 | ✅ 18557 ms (15 iters) | ✅ 18149 ms (15 iters) | ≈ -2.2% (±5%) | — / — |
Internal counter details (ddprof)
ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
| Benchmark | JDK | Dropped rec | Dropped jvmti | Dropped trace | Skipped WC | AGCT fail | Unwind fail |
|---|---|---|---|---|---|---|---|
| akka-uct | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 3 | 1933 / 1878 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 3 | 2168 / 2192 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 3 / 3 | 8581 / 8674 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 4 / 2 | 8699 / 8624 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 2 | 1270 / 1250 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 1288 / 1273 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 3 | 3024 / 3018 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 4 / 1 | 2773 / 2909 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 6 / 4 | 3520 / 3466 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 6 / 4 | 3468 / 3501 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | 1593 / 1778 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 1904 / 1807 | ✅ / ✅ | ✅ / ✅ |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
ddprof-lib/src/main/cpp/vmEntry.cpp:593
- VM::ready() can be invoked multiple times (initLibrary(), initProfilerBridge(), and again from the VMInit callback). Currently it calls VMStructs::init(lib) on every invocation, but VMStructs::init() is not idempotent: it appends to static caches like VMStructs::_unsafe_to_walk via CodeCache::add(), so repeated init will accumulate duplicates and allocate memory unnecessarily. Guard VMStructs::init() so it runs only once (the comment above also claims it only runs once, but the code doesn’t enforce that).
JitWriteProtection jit(true);
CodeCache* lib = openJvmLibrary();
assert(lib != nullptr && "JVM library must have been loaded");
// Initialize VMStructs
VMStructs::init(lib);
Contributor
Benchmark Results (commit a5ec8f8)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126615747 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10278 ms (21 iters) | ✅ 10275 ms (21 iters) | ≈ -0% (±11.1%) | — / — |
| akka-uct | 25 | ✅ 8716 ms (24 iters) | ✅ 8825 ms (24 iters) | ≈ +1.3% (±10.4%) | — / — |
| finagle-chirper | 21 | ✅ 5949 ms (33 iters) | ✅ 5997 ms (33 iters) | ≈ +0.8% (±25.3%) | |
| finagle-chirper | 25 | ✅ 5479 ms (36 iters) | ✅ 5464 ms (36 iters) | ≈ -0.3% (±24.2%) | |
| fj-kmeans | 21 | ✅ 2838 ms (66 iters) | ✅ 2753 ms (68 iters) | 🟢 -3% | — / — |
| fj-kmeans | 25 | ✅ 2824 ms (66 iters) | ✅ 2811 ms (66 iters) | ≈ -0.5% (±2.6%) | — / — |
| future-genetic | 21 | ✅ 2103 ms (88 iters) | ✅ 2030 ms (91 iters) | 🟢 -3.5% | — / — |
| future-genetic | 25 | ✅ 2091 ms (89 iters) | ✅ 2136 ms (87 iters) | ≈ +2.2% (±2.5%) | — / — |
| naive-bayes | 21 | ✅ 1288 ms (133 iters) | ✅ 1298 ms (132 iters) | ≈ +0.8% (±32.6%) | — / — |
| naive-bayes | 25 | ✅ 1021 ms (168 iters) | ✅ 1011 ms (169 iters) | ≈ -1% (±31.3%) | — / — |
| reactors | 21 | ✅ 15739 ms (15 iters) | ✅ 16648 ms (15 iters) | ≈ +5.8% (±7.8%) | — / — |
| reactors | 25 | ✅ 18232 ms (15 iters) | ✅ 18879 ms (15 iters) | ≈ +3.5% (±5.3%) | — / — |
Internal counter details (ddprof)
ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
| Benchmark | JDK | Dropped rec | Dropped jvmti | Dropped trace | Skipped WC | AGCT fail | Unwind fail |
|---|---|---|---|---|---|---|---|
| akka-uct | 21 | ✅ / ✅ | ✅ / ✅ | 2 / 2 | 1980 / 1958 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 1 | 2393 / 2377 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 3 / 7 | 8401 / 8293 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 8206 / 8469 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 1 | 1256 / 1259 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 2 / ✅ | 1282 / 1270 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 3022 / 2967 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 1 | 2886 / 2947 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 4 | 3554 / 3497 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 6 / 3 | 3497 / 3481 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 1504 / 1631 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 1857 / 1968 | ✅ / ✅ | ✅ / ✅ |
jbachorik
approved these changes
Jul 24, 2026
jbachorik
left a comment
jbachorik
left a comment
Collaborator
There was a problem hiding this comment.
Looking good! Great to see the hardening effort bearing fruits!
Contributor
Author
|
Thanks for the review. |
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: queued
labels
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 does this PR do?:
This PR targets fault-injection-related startup stability in the native HotSpot integration by reordering VMStructs initialization relative to signal-handler installation and by making
VMFlag::name()safer to call when crash protection isn’t active.Changes:
Move
VMStructs::init()from early shared initialization into the laterVM::ready()path (afterProfiler::setupSignalHandlers()).Add SafeAccess-backed pointer loading helpers in vmStructs.inline.h and use them to harden
VMFlag::name().Minor refactor in
VMFlag::find()to avoid calling name() twice.Motivation:
Harden code to avoid crashes due to access bad memory.
Additional Notes:
How to test the change?:
For Datadog employees:
credentials of any kind, I've requested a security review (run the
dd:platform-security-reviewskill, or file a request via the PSEC review form).
bewairealso runs automatically on every PR.Unsure? Have a question? Request a review!