Do not use java.nio in crashtracking init (premain)#11080
Do not use java.nio in crashtracking init (premain)#11080
Conversation
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
| LOGGER.warn(SEND_TELEMETRY, "Failed deleting config file: {}", cfgPath, e); | ||
| } | ||
| LOGGER.debug("Deleting config file: {}", cfgFile); | ||
| cfgFile.delete(); |
There was a problem hiding this comment.
Silent failure on config file deletion
The original Files.deleteIfExists() call logged a WARN with SEND_TELEMETRY on failure. The replacement cfgFile.delete() returns false on failure but the return value is unchecked, silently dropping that telemetry signal.
| cfgFile.delete(); | |
| LOGGER.debug("Deleting config file: {}", cfgFile); | |
| if (!cfgFile.delete() && cfgFile.exists()) { | |
| LOGGER.warn(SEND_TELEMETRY, "Failed deleting config file: {}", cfgFile); | |
| } |
| scriptDirectory); | ||
| scriptDirectory.setReadable(true, false); | ||
| scriptDirectory.setWritable(true, false); | ||
| scriptDirectory.setExecutable(true, false); |
There was a problem hiding this comment.
Return values of permission calls are ignored
setReadable(), setWritable(), and setExecutable() all return boolean indicating success, but the return values are not checked here (or in the analogous block in OOMENotifierScriptInitializer). A silent permission failure can cause hard-to-diagnose downstream crashes.
| scriptDirectory.setExecutable(true, false); | |
| if (!scriptDirectory.setReadable(true, false) | |
| || !scriptDirectory.setWritable(true, false) | |
| || !scriptDirectory.setExecutable(true, false)) { | |
| LOG.warn( | |
| SEND_TELEMETRY, | |
| "Failed to set permissions on crash tracking script folder {}. {}", | |
| scriptDirectory, SETUP_FAILURE_MESSAGE); | |
| } |
| // ignore | ||
| } | ||
| LOGGER.warn(SEND_TELEMETRY, "Failed writing config file: {}", cfgFile); | ||
| cfgFile.delete(); |
There was a problem hiding this comment.
Undocumented best-effort delete
The original code had an explicit catch (IOException ignored) { } that documented the intent to swallow the error. The replacement is also silent, but without that comment future readers may assume this is an oversight.
| cfgFile.delete(); | |
| cfgFile.delete(); // best-effort cleanup; failure is acceptable here |
| File scriptFile, String onErrorFile, String agentJar) { | ||
| File scriptDirectory = scriptFile.getParentFile(); | ||
| if (!scriptDirectory.exists()) { | ||
| if (!scriptDirectory.mkdirs()) { |
There was a problem hiding this comment.
Non-atomic directory creation and permission setting
The original Files.createDirectories() with PosixFilePermissions set permissions atomically on creation. The replacement mkdirs() followed by separate setReadable/setWritable/setExecutable calls introduces a race window between directory creation and permission application.
This is init-time code so the practical impact is minimal, but worth documenting the accepted trade-off with a comment.
BenchmarksStartupParameters
See matching parameters
SummaryFound 3 performance improvements and 0 performance regressions! Performance is the same for 56 metrics, 12 unstable metrics.
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.62.0-SNAPSHOT~aba8a0e469, baseline=1.62.0-SNAPSHOT~742623564b
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.056 s) : 0, 1055783
Total [baseline] (8.859 s) : 0, 8858779
Agent [candidate] (1.065 s) : 0, 1065260
Total [candidate] (8.821 s) : 0, 8820780
section iast
Agent [baseline] (1.226 s) : 0, 1225544
Total [baseline] (9.552 s) : 0, 9551889
Agent [candidate] (1.218 s) : 0, 1218296
Total [candidate] (9.519 s) : 0, 9518772
gantt
title insecure-bank - break down per module: candidate=1.62.0-SNAPSHOT~aba8a0e469, baseline=1.62.0-SNAPSHOT~742623564b
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.222 ms) : 0, 1222
crashtracking [candidate] (1.227 ms) : 0, 1227
BytebuddyAgent [baseline] (631.516 ms) : 0, 631516
BytebuddyAgent [candidate] (638.956 ms) : 0, 638956
AgentMeter [baseline] (29.379 ms) : 0, 29379
AgentMeter [candidate] (29.606 ms) : 0, 29606
GlobalTracer [baseline] (248.203 ms) : 0, 248203
GlobalTracer [candidate] (250.477 ms) : 0, 250477
AppSec [baseline] (32.086 ms) : 0, 32086
AppSec [candidate] (31.976 ms) : 0, 31976
Debugger [baseline] (59.432 ms) : 0, 59432
Debugger [candidate] (59.13 ms) : 0, 59130
Remote Config [baseline] (597.956 µs) : 0, 598
Remote Config [candidate] (598.109 µs) : 0, 598
Telemetry [baseline] (8.123 ms) : 0, 8123
Telemetry [candidate] (8.109 ms) : 0, 8109
Flare Poller [baseline] (9.198 ms) : 0, 9198
Flare Poller [candidate] (8.945 ms) : 0, 8945
section iast
crashtracking [baseline] (1.25 ms) : 0, 1250
crashtracking [candidate] (1.193 ms) : 0, 1193
BytebuddyAgent [baseline] (802.67 ms) : 0, 802670
BytebuddyAgent [candidate] (797.309 ms) : 0, 797309
AgentMeter [baseline] (11.408 ms) : 0, 11408
AgentMeter [candidate] (11.346 ms) : 0, 11346
GlobalTracer [baseline] (239.797 ms) : 0, 239797
GlobalTracer [candidate] (238.377 ms) : 0, 238377
AppSec [baseline] (30.131 ms) : 0, 30131
AppSec [candidate] (31.673 ms) : 0, 31673
Debugger [baseline] (61.703 ms) : 0, 61703
Debugger [candidate] (57.437 ms) : 0, 57437
Remote Config [baseline] (541.335 µs) : 0, 541
Remote Config [candidate] (491.919 µs) : 0, 492
Telemetry [baseline] (11.42 ms) : 0, 11420
Telemetry [candidate] (14.928 ms) : 0, 14928
Flare Poller [baseline] (3.443 ms) : 0, 3443
Flare Poller [candidate] (3.455 ms) : 0, 3455
IAST [baseline] (26.712 ms) : 0, 26712
IAST [candidate] (25.688 ms) : 0, 25688
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.62.0-SNAPSHOT~aba8a0e469, baseline=1.62.0-SNAPSHOT~742623564b
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.054 s) : 0, 1053849
Total [baseline] (11.056 s) : 0, 11056236
Agent [candidate] (1.061 s) : 0, 1060911
Total [candidate] (11.141 s) : 0, 11141363
section appsec
Agent [baseline] (1.247 s) : 0, 1246899
Total [baseline] (11.091 s) : 0, 11091372
Agent [candidate] (1.25 s) : 0, 1250354
Total [candidate] (11.09 s) : 0, 11090219
section iast
Agent [baseline] (1.232 s) : 0, 1232005
Total [baseline] (11.384 s) : 0, 11383740
Agent [candidate] (1.224 s) : 0, 1224280
Total [candidate] (11.245 s) : 0, 11245102
section profiling
Agent [baseline] (1.192 s) : 0, 1192420
Total [baseline] (11.243 s) : 0, 11243164
Agent [candidate] (1.184 s) : 0, 1183544
Total [candidate] (11.067 s) : 0, 11066802
gantt
title petclinic - break down per module: candidate=1.62.0-SNAPSHOT~aba8a0e469, baseline=1.62.0-SNAPSHOT~742623564b
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.218 ms) : 0, 1218
crashtracking [candidate] (1.223 ms) : 0, 1223
BytebuddyAgent [baseline] (631.573 ms) : 0, 631573
BytebuddyAgent [candidate] (633.754 ms) : 0, 633754
AgentMeter [baseline] (29.436 ms) : 0, 29436
AgentMeter [candidate] (29.701 ms) : 0, 29701
GlobalTracer [baseline] (248.139 ms) : 0, 248139
GlobalTracer [candidate] (250.876 ms) : 0, 250876
AppSec [baseline] (31.91 ms) : 0, 31910
AppSec [candidate] (32.337 ms) : 0, 32337
Debugger [baseline] (60.104 ms) : 0, 60104
Debugger [candidate] (60.717 ms) : 0, 60717
Remote Config [baseline] (599.803 µs) : 0, 600
Remote Config [candidate] (603.731 µs) : 0, 604
Telemetry [baseline] (8.084 ms) : 0, 8084
Telemetry [candidate] (8.163 ms) : 0, 8163
Flare Poller [baseline] (6.761 ms) : 0, 6761
Flare Poller [candidate] (7.416 ms) : 0, 7416
section appsec
crashtracking [baseline] (1.215 ms) : 0, 1215
crashtracking [candidate] (1.226 ms) : 0, 1226
BytebuddyAgent [baseline] (661.017 ms) : 0, 661017
BytebuddyAgent [candidate] (663.846 ms) : 0, 663846
AgentMeter [baseline] (12.06 ms) : 0, 12060
AgentMeter [candidate] (12.134 ms) : 0, 12134
GlobalTracer [baseline] (248.667 ms) : 0, 248667
GlobalTracer [candidate] (249.339 ms) : 0, 249339
IAST [baseline] (24.481 ms) : 0, 24481
IAST [candidate] (24.625 ms) : 0, 24625
AppSec [baseline] (184.355 ms) : 0, 184355
AppSec [candidate] (184.188 ms) : 0, 184188
Debugger [baseline] (66.006 ms) : 0, 66006
Debugger [candidate] (65.798 ms) : 0, 65798
Remote Config [baseline] (607.207 µs) : 0, 607
Remote Config [candidate] (598.209 µs) : 0, 598
Telemetry [baseline] (8.624 ms) : 0, 8624
Telemetry [candidate] (8.626 ms) : 0, 8626
Flare Poller [baseline] (3.549 ms) : 0, 3549
Flare Poller [candidate] (3.53 ms) : 0, 3530
section iast
crashtracking [baseline] (1.24 ms) : 0, 1240
crashtracking [candidate] (1.214 ms) : 0, 1214
BytebuddyAgent [baseline] (808.584 ms) : 0, 808584
BytebuddyAgent [candidate] (801.275 ms) : 0, 801275
AgentMeter [baseline] (11.518 ms) : 0, 11518
AgentMeter [candidate] (11.367 ms) : 0, 11367
GlobalTracer [baseline] (239.042 ms) : 0, 239042
GlobalTracer [candidate] (239.557 ms) : 0, 239557
IAST [baseline] (25.734 ms) : 0, 25734
IAST [candidate] (25.768 ms) : 0, 25768
AppSec [baseline] (32.621 ms) : 0, 32621
AppSec [candidate] (30.845 ms) : 0, 30845
Debugger [baseline] (59.36 ms) : 0, 59360
Debugger [candidate] (59.464 ms) : 0, 59464
Remote Config [baseline] (1.099 ms) : 0, 1099
Remote Config [candidate] (522.418 µs) : 0, 522
Telemetry [baseline] (12.883 ms) : 0, 12883
Telemetry [candidate] (14.719 ms) : 0, 14719
Flare Poller [baseline] (3.485 ms) : 0, 3485
Flare Poller [candidate] (3.552 ms) : 0, 3552
section profiling
crashtracking [baseline] (1.188 ms) : 0, 1188
crashtracking [candidate] (1.173 ms) : 0, 1173
BytebuddyAgent [baseline] (695.69 ms) : 0, 695690
BytebuddyAgent [candidate] (691.216 ms) : 0, 691216
AgentMeter [baseline] (9.159 ms) : 0, 9159
AgentMeter [candidate] (8.816 ms) : 0, 8816
GlobalTracer [baseline] (208.491 ms) : 0, 208491
GlobalTracer [candidate] (207.849 ms) : 0, 207849
AppSec [baseline] (32.772 ms) : 0, 32772
AppSec [candidate] (32.125 ms) : 0, 32125
Debugger [baseline] (66.038 ms) : 0, 66038
Debugger [candidate] (64.792 ms) : 0, 64792
Remote Config [baseline] (572.808 µs) : 0, 573
Remote Config [candidate] (565.399 µs) : 0, 565
Telemetry [baseline] (7.87 ms) : 0, 7870
Telemetry [candidate] (8.598 ms) : 0, 8598
Flare Poller [baseline] (3.567 ms) : 0, 3567
Flare Poller [candidate] (3.478 ms) : 0, 3478
ProfilingAgent [baseline] (95.382 ms) : 0, 95382
ProfilingAgent [candidate] (93.733 ms) : 0, 93733
Profiling [baseline] (95.971 ms) : 0, 95971
Profiling [candidate] (94.302 ms) : 0, 94302
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 1 performance regressions! Performance is the same for 20 metrics, 15 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~aba8a0e469, baseline=1.62.0-SNAPSHOT~742623564b
dateFormat X
axisFormat %s
section baseline
no_agent (1.259 ms) : 1247, 1272
. : milestone, 1259,
iast (3.219 ms) : 3173, 3264
. : milestone, 3219,
iast_FULL (6.186 ms) : 6122, 6250
. : milestone, 6186,
iast_GLOBAL (3.718 ms) : 3649, 3787
. : milestone, 3718,
profiling (2.078 ms) : 2060, 2096
. : milestone, 2078,
tracing (1.937 ms) : 1918, 1956
. : milestone, 1937,
section candidate
no_agent (1.243 ms) : 1231, 1255
. : milestone, 1243,
iast (3.244 ms) : 3199, 3290
. : milestone, 3244,
iast_FULL (5.961 ms) : 5901, 6021
. : milestone, 5961,
iast_GLOBAL (3.715 ms) : 3662, 3768
. : milestone, 3715,
profiling (2.188 ms) : 2167, 2208
. : milestone, 2188,
tracing (1.868 ms) : 1852, 1884
. : milestone, 1868,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~aba8a0e469, baseline=1.62.0-SNAPSHOT~742623564b
dateFormat X
axisFormat %s
section baseline
no_agent (17.95 ms) : 17769, 18132
. : milestone, 17950,
appsec (18.11 ms) : 17926, 18294
. : milestone, 18110,
code_origins (17.986 ms) : 17807, 18166
. : milestone, 17986,
iast (18.876 ms) : 18685, 19068
. : milestone, 18876,
profiling (18.006 ms) : 17828, 18184
. : milestone, 18006,
tracing (17.79 ms) : 17614, 17966
. : milestone, 17790,
section candidate
no_agent (17.288 ms) : 17115, 17462
. : milestone, 17288,
appsec (18.725 ms) : 18537, 18914
. : milestone, 18725,
code_origins (17.561 ms) : 17388, 17733
. : milestone, 17561,
iast (18.613 ms) : 18430, 18796
. : milestone, 18613,
profiling (18.017 ms) : 17838, 18195
. : milestone, 18017,
tracing (17.957 ms) : 17781, 18134
. : milestone, 17957,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 1 performance regressions! Performance is the same for 10 metrics, 1 unstable metrics.
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~aba8a0e469, baseline=1.62.0-SNAPSHOT~742623564b
dateFormat X
axisFormat %s
section baseline
no_agent (1.489 ms) : 1477, 1500
. : milestone, 1489,
appsec (3.827 ms) : 3607, 4048
. : milestone, 3827,
iast (2.272 ms) : 2203, 2341
. : milestone, 2272,
iast_GLOBAL (2.188 ms) : 2125, 2250
. : milestone, 2188,
profiling (2.09 ms) : 2035, 2145
. : milestone, 2090,
tracing (2.073 ms) : 2020, 2127
. : milestone, 2073,
section candidate
no_agent (1.483 ms) : 1471, 1494
. : milestone, 1483,
appsec (3.822 ms) : 3601, 4043
. : milestone, 3822,
iast (2.27 ms) : 2201, 2339
. : milestone, 2270,
iast_GLOBAL (2.314 ms) : 2245, 2384
. : milestone, 2314,
profiling (2.106 ms) : 2051, 2162
. : milestone, 2106,
tracing (2.09 ms) : 2036, 2144
. : milestone, 2090,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~aba8a0e469, baseline=1.62.0-SNAPSHOT~742623564b
dateFormat X
axisFormat %s
section baseline
no_agent (14.953 s) : 14953000, 14953000
. : milestone, 14953000,
appsec (14.687 s) : 14687000, 14687000
. : milestone, 14687000,
iast (18.424 s) : 18424000, 18424000
. : milestone, 18424000,
iast_GLOBAL (17.946 s) : 17946000, 17946000
. : milestone, 17946000,
profiling (14.722 s) : 14722000, 14722000
. : milestone, 14722000,
tracing (14.896 s) : 14896000, 14896000
. : milestone, 14896000,
section candidate
no_agent (14.851 s) : 14851000, 14851000
. : milestone, 14851000,
appsec (14.826 s) : 14826000, 14826000
. : milestone, 14826000,
iast (18.491 s) : 18491000, 18491000
. : milestone, 18491000,
iast_GLOBAL (18.132 s) : 18132000, 18132000
. : milestone, 18132000,
profiling (14.892 s) : 14892000, 14892000
. : milestone, 14892000,
tracing (14.883 s) : 14883000, 14883000
. : milestone, 14883000,
|
What Does This Do
Avoid using
java.nioin the initialisation part of the crashtracking agent. This because, at this point, initializing java.nio can lead to side effects (deadlocks, premature initialisations, etc..)Also, the same operations can be done using a regular java.io
Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.