[fault-injection] Startup should setup signal handlers before initializing VMStructs and protect VMFlag::name() call#674
[fault-injection] Startup should setup signal handlers before initializing VMStructs and protect VMFlag::name() call#674zhengyu123 wants to merge 19 commits into
Conversation
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.
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):
|
Reliability & Chaos Results✅ All reliability & chaos checks passed Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/126272718 |
CI Test ResultsRun: #29931516459 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Failed Testsglibc-aarch64/debug / 25-graalJob: View logs No detailed failure information available. Check the job logs. Summary: Total: 32 | Passed: 31 | Failed: 1 Updated: 2026-07-22 15:30:05 UTC |
|
🔄 Datadog auto-retried 1 job - 1 passed on retry 🔗 Commit SHA: a546a5c | Docs | Datadog PR Page | Give us feedback! |
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>
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>
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 | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ |
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
*/
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>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8da863ca70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Benchmark Results (commit 8da863c)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126089137 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):
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8cdcf6025
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Benchmark Results (commit f8cdcf6)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126109408 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10273 ms (21 iters) | ✅ 10342 ms (21 iters) | ≈ +0.7% (±10.9%) | — / — |
| akka-uct | 25 | ✅ 8811 ms (24 iters) | ✅ 8872 ms (24 iters) | ≈ +0.7% (±9.9%) | — / — |
| finagle-chirper | 21 | ✅ 5974 ms (33 iters) | ✅ 5977 ms (33 iters) | ≈ +0.1% (±25.1%) | |
| finagle-chirper | 25 | ✅ 5475 ms (36 iters) | ✅ 5427 ms (36 iters) | ≈ -0.9% (±23.9%) | |
| fj-kmeans | 21 | ✅ 2771 ms (68 iters) | ✅ 2757 ms (68 iters) | ≈ -0.5% (±2.7%) | — / — |
| fj-kmeans | 25 | ✅ 2813 ms (66 iters) | ✅ 2808 ms (66 iters) | ≈ -0.2% (±2.6%) | — / — |
| future-genetic | 21 | ✅ 2070 ms (89 iters) | ✅ 2070 ms (89 iters) | ≈ 0% (±2.6%) | — / — |
| future-genetic | 25 | ✅ 2048 ms (91 iters) | ✅ 2112 ms (88 iters) | 🔴 +3.1% | — / — |
| naive-bayes | 21 | ✅ 1305 ms (131 iters) | ✅ 1246 ms (137 iters) | ≈ -4.5% (±32.1%) | — / — |
| naive-bayes | 25 | ✅ 997 ms (172 iters) | ✅ 972 ms (175 iters) | ≈ -2.5% (±31.1%) | — / — |
| reactors | 21 | ✅ 16576 ms (15 iters) | ✅ 15766 ms (15 iters) | ≈ -4.9% (±7.1%) | — / — |
| reactors | 25 | ✅ 18512 ms (15 iters) | ✅ 18805 ms (15 iters) | ≈ +1.6% (±5.2%) | — / — |
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 | ✅ / ✅ | ✅ / ✅ | 6 / ✅ | 2034 / 1916 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 3 | 2340 / 2435 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 4 | 8770 / 8573 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 2 / ✅ | 8203 / 8283 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 3 / 3 | 1271 / 1304 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 1300 / 1260 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 4 | 2945 / 2923 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 2904 / 2944 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 6 / 2 | 3485 / 3542 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 3493 / 3533 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 1742 / 1593 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 2005 / 1911 | ✅ / ✅ | ✅ / ✅ |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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:214
- Avoid casting the function pointer
_asyncGetCallTracetovoid*for the null check. Converting function pointers to object pointers is non-portable/UB in standard C++ and can trigger compiler warnings; a direct comparison tonullptris well-defined.
if ((void*)_asyncGetCallTrace == nullptr) {
return nullptr;
}
…iler into zgu/unprotected_vmflag
Benchmark Results (commit eefd7ae)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126118917 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):
|
Benchmark Results (commit a546a5c)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126272736 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):
|
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!