Skip to content

Commit aeb83d6

Browse files
amarzialidevflow.devflow-routing-intake
amarziali
and
devflow.devflow-routing-intake
authored
Do not use java.nio in crashtracking init (premain) (#11080)
# What Does This Do Avoid using `java.nio` in 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 - Format the title according to [the contribution guidelines](https://github.com/DataDog/dd-trace-java/blob/master/CONTRIBUTING.md#title-format) - Assign the `type:` and (`comp:` or `inst:`) labels in addition to [any other useful labels](https://github.com/DataDog/dd-trace-java/blob/master/CONTRIBUTING.md#labels) - Avoid using `close`, `fix`, or [any linking keywords](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword) when referencing an issue Use `solves` instead, and assign the PR [milestone](https://github.com/DataDog/dd-trace-java/milestones) to the issue - Update the [CODEOWNERS](https://github.com/DataDog/dd-trace-java/blob/master/.github/CODEOWNERS) file on source file addition, migration, or deletion - Update [public documentation](https://docs.datadoghq.com/tracing/trace_collection/library_config/java/) with any new configuration flags or behaviors Jira ticket: [PROJ-IDENT] ***Note:*** **Once your PR is ready to merge, add it to the merge queue by commenting `/merge`.** `/merge -c` cancels 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](https://datadoghq.atlassian.net/wiki/spaces/DEVX/pages/3121612126/MergeQueue). <!-- # Opening vs Drafting a PR: When opening a pull request, please open it as a draft to not auto assign reviewers before you feel the pull request is in a reviewable state. # Linking a JIRA ticket: Please link your JIRA ticket by adding its identifier between brackets (ex [PROJ-IDENT]) in the PR description, not the title. This requirement only applies to Datadog employees. --> Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent 995f760 commit aeb83d6

6 files changed

Lines changed: 147 additions & 193 deletions

File tree

‎dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/ConfigManager.java‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,13 @@
1212
import datadog.trace.util.RandomUtils;
1313
import java.io.BufferedReader;
1414
import java.io.BufferedWriter;
15+
import java.io.File;
16+
import java.io.FileInputStream;
17+
import java.io.FileOutputStream;
1518
import java.io.IOException;
16-
import java.nio.file.Files;
17-
import java.nio.file.Path;
19+
import java.io.InputStreamReader;
20+
import java.io.OutputStreamWriter;
21+
import java.nio.charset.StandardCharsets;
1822
import java.util.regex.Pattern;
1923
import java.util.stream.Collectors;
2024
import javax.annotation.Nullable;
@@ -157,8 +161,8 @@ public StoredConfig build() {
157161

158162
private ConfigManager() {}
159163

160-
private static String getBaseName(Path path) {
161-
String filename = path.getFileName().toString();
164+
private static String getBaseName(File file) {
165+
String filename = file.getName();
162166
int dotIndex = filename.lastIndexOf('.');
163167
if (dotIndex == -1) {
164168
return filename;
@@ -185,18 +189,20 @@ private static void writeEntry(BufferedWriter writer, CharSequence key, CharSequ
185189
writer.newLine();
186190
}
187191

188-
public static void writeConfigToPath(Path scriptPath, String... additionalEntries) {
189-
String cfgFileName = getBaseName(scriptPath) + PID_PREFIX + PidHelper.getPid() + ".cfg";
190-
Path cfgPath = scriptPath.resolveSibling(cfgFileName);
191-
writeConfigToFile(Config.get(), cfgPath, additionalEntries);
192+
public static void writeConfigToPath(File scriptFile, String... additionalEntries) {
193+
String cfgFileName = getBaseName(scriptFile) + PID_PREFIX + PidHelper.getPid() + ".cfg";
194+
File cfgFile = new File(scriptFile.getParentFile(), cfgFileName);
195+
writeConfigToFile(Config.get(), cfgFile, additionalEntries);
192196
}
193197

194198
// @VisibleForTesting
195-
static void writeConfigToFile(Config config, Path cfgPath, String... additionalEntries) {
199+
static void writeConfigToFile(Config config, File cfgFile, String... additionalEntries) {
196200
final WellKnownTags wellKnownTags = config.getWellKnownTags();
197201

198-
LOGGER.debug("Writing config file: {}", cfgPath);
199-
try (BufferedWriter bw = Files.newBufferedWriter(cfgPath)) {
202+
LOGGER.debug("Writing config file: {}", cfgFile);
203+
try (BufferedWriter bw =
204+
new BufferedWriter(
205+
new OutputStreamWriter(new FileOutputStream(cfgFile), StandardCharsets.UTF_8))) {
200206
for (int i = 0; i < additionalEntries.length; i += 2) {
201207
writeEntry(bw, additionalEntries[i], additionalEntries[i + 1]);
202208
}
@@ -217,27 +223,21 @@ static void writeConfigToFile(Config config, Path cfgPath, String... additionalE
217223
new Thread(
218224
AGENT_THREAD_GROUP,
219225
() -> {
220-
try {
221-
LOGGER.debug("Deleting config file: {}", cfgPath);
222-
Files.deleteIfExists(cfgPath);
223-
} catch (IOException e) {
224-
LOGGER.warn(SEND_TELEMETRY, "Failed deleting config file: {}", cfgPath, e);
225-
}
226+
LOGGER.debug("Deleting config file: {}", cfgFile);
227+
cfgFile.delete();
226228
}));
227-
LOGGER.debug("Config file written: {}", cfgPath);
229+
LOGGER.debug("Config file written: {}", cfgFile);
228230
} catch (IOException e) {
229-
LOGGER.warn(SEND_TELEMETRY, "Failed writing config file: {}", cfgPath);
230-
try {
231-
Files.deleteIfExists(cfgPath);
232-
} catch (IOException ignored) {
233-
// ignore
234-
}
231+
LOGGER.warn(SEND_TELEMETRY, "Failed writing config file: {}", cfgFile);
232+
cfgFile.delete(); // best-effort cleanup; failure is acceptable here
235233
}
236234
}
237235

238236
@Nullable
239-
public static StoredConfig readConfig(Config config, Path scriptPath) {
240-
try (final BufferedReader reader = Files.newBufferedReader(scriptPath)) {
237+
public static StoredConfig readConfig(Config config, File scriptFile) {
238+
try (final BufferedReader reader =
239+
new BufferedReader(
240+
new InputStreamReader(new FileInputStream(scriptFile), StandardCharsets.UTF_8))) {
241241
final StoredConfig.Builder cfgBuilder = new StoredConfig.Builder(config);
242242
String line;
243243
while ((line = reader.readLine()) != null) {
@@ -284,7 +284,7 @@ public static StoredConfig readConfig(Config config, Path scriptPath) {
284284
}
285285
return cfgBuilder.build();
286286
} catch (Throwable t) {
287-
LOGGER.error("Failed to read config file: {}", scriptPath, t);
287+
LOGGER.error("Failed to read config file: {}", scriptFile, t);
288288
}
289289
return null;
290290
}

‎dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java‎

Lines changed: 42 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,23 @@
22

33
import static datadog.crashtracking.ConfigManager.writeConfigToPath;
44
import static datadog.crashtracking.Initializer.LOG;
5-
import static datadog.crashtracking.Initializer.RWXRWXRWX;
6-
import static datadog.crashtracking.Initializer.R_XR_XR_X;
75
import static datadog.crashtracking.Initializer.findAgentJar;
86
import static datadog.crashtracking.Initializer.getCrashUploaderTemplate;
97
import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY;
10-
import static java.nio.file.attribute.PosixFilePermissions.asFileAttribute;
11-
import static java.nio.file.attribute.PosixFilePermissions.fromString;
128
import static java.util.Locale.ROOT;
139

1410
import datadog.environment.SystemProperties;
1511
import datadog.trace.util.PidHelper;
1612
import datadog.trace.util.Strings;
1713
import java.io.BufferedReader;
1814
import java.io.BufferedWriter;
15+
import java.io.File;
16+
import java.io.FileOutputStream;
1917
import java.io.IOException;
2018
import java.io.InputStream;
2119
import java.io.InputStreamReader;
22-
import java.nio.file.FileAlreadyExistsException;
23-
import java.nio.file.Files;
24-
import java.nio.file.Path;
25-
import java.nio.file.Paths;
20+
import java.io.OutputStreamWriter;
21+
import java.nio.charset.StandardCharsets;
2622

2723
public final class CrashUploaderScriptInitializer {
2824
private static final String SETUP_FAILURE_MESSAGE = "Crash tracking will not work properly.";
@@ -54,71 +50,78 @@ static void initialize(String onErrorVal, String onErrorFile, String javacorePat
5450
return;
5551
}
5652

57-
Path scriptPath = Paths.get(onErrorVal.replace(" %p", ""));
53+
File scriptFile = new File(onErrorVal.replace(" %p", ""));
5854
boolean isDDCrashUploader =
59-
scriptPath.getFileName().toString().toLowerCase(ROOT).contains("dd_crash_uploader");
60-
if (isDDCrashUploader && !copyCrashUploaderScript(scriptPath, onErrorFile, agentJar)) {
55+
scriptFile.getName().toLowerCase(ROOT).contains("dd_crash_uploader");
56+
if (isDDCrashUploader && !copyCrashUploaderScript(scriptFile, onErrorFile, agentJar)) {
6157
return;
6258
}
6359

6460
if (javacorePath != null && !javacorePath.isEmpty()) {
65-
writeConfigToPath(scriptPath, "agent", agentJar, "javacore_path", javacorePath);
61+
writeConfigToPath(scriptFile, "agent", agentJar, "javacore_path", javacorePath);
6662
} else {
67-
writeConfigToPath(scriptPath, "agent", agentJar, "hs_err", onErrorFile);
63+
writeConfigToPath(scriptFile, "agent", agentJar, "hs_err", onErrorFile);
6864
}
6965
}
7066

7167
private static boolean copyCrashUploaderScript(
72-
Path scriptPath, String onErrorFile, String agentJar) {
73-
Path scriptDirectory = scriptPath.getParent();
74-
try {
75-
Files.createDirectories(scriptDirectory, asFileAttribute(fromString(RWXRWXRWX)));
76-
} catch (UnsupportedOperationException e) {
77-
LOG.warn(
78-
SEND_TELEMETRY,
79-
"Unsupported permissions '" + RWXRWXRWX + "' for {}. " + SETUP_FAILURE_MESSAGE,
80-
scriptDirectory);
81-
return false;
82-
} catch (FileAlreadyExistsException ignored) {
83-
// can be safely ignored; if the folder exists we will just reuse it
84-
if (!Files.isWritable(scriptDirectory)) {
68+
File scriptFile, String onErrorFile, String agentJar) {
69+
File scriptDirectory = scriptFile.getParentFile();
70+
if (!scriptDirectory.exists()) {
71+
if (!scriptDirectory.mkdirs()) {
8572
LOG.warn(
86-
SEND_TELEMETRY, "Read only directory {}. " + SETUP_FAILURE_MESSAGE, scriptDirectory);
73+
SEND_TELEMETRY,
74+
"Failed to create writable crash tracking script folder {}. " + SETUP_FAILURE_MESSAGE,
75+
scriptDirectory);
8776
return false;
8877
}
89-
} catch (IOException e) {
90-
LOG.warn(
91-
SEND_TELEMETRY,
92-
"Failed to create writable crash tracking script folder {}. " + SETUP_FAILURE_MESSAGE,
93-
scriptDirectory);
78+
boolean permissionFailure = false;
79+
permissionFailure |= !scriptDirectory.setReadable(true, false);
80+
permissionFailure |= !scriptDirectory.setWritable(true, false);
81+
permissionFailure |= !scriptDirectory.setExecutable(true, false);
82+
if (permissionFailure) {
83+
LOG.warn(
84+
SEND_TELEMETRY,
85+
"Failed to set permissions on crash tracking script folder {}. {}",
86+
scriptDirectory,
87+
SETUP_FAILURE_MESSAGE);
88+
}
89+
}
90+
if (!scriptDirectory.canWrite()) {
91+
LOG.warn(SEND_TELEMETRY, "Read only directory {}. " + SETUP_FAILURE_MESSAGE, scriptDirectory);
9492
return false;
9593
}
9694
try {
97-
LOG.debug("Writing crash uploader script: {}", scriptPath);
98-
writeCrashUploaderScript(getCrashUploaderTemplate(), scriptPath, agentJar, onErrorFile);
95+
LOG.debug("Writing crash uploader script: {}", scriptFile);
96+
writeCrashUploaderScript(getCrashUploaderTemplate(), scriptFile, agentJar, onErrorFile);
9997
} catch (IOException e) {
10098
LOG.warn(
10199
SEND_TELEMETRY,
102100
"Failed to copy crash tracking script {}. " + SETUP_FAILURE_MESSAGE,
103-
scriptPath);
101+
scriptFile);
104102
return false;
105103
}
106104
return true;
107105
}
108106

109107
private static void writeCrashUploaderScript(
110-
InputStream template, Path scriptPath, String execClass, String crashFile)
108+
InputStream template, File scriptFile, String execClass, String crashFile)
111109
throws IOException {
112-
if (!Files.exists(scriptPath)) {
110+
if (!scriptFile.exists()) {
113111
try (BufferedReader br = new BufferedReader(new InputStreamReader(template));
114-
BufferedWriter bw = Files.newBufferedWriter(scriptPath)) {
112+
BufferedWriter bw =
113+
new BufferedWriter(
114+
new OutputStreamWriter(
115+
new FileOutputStream(scriptFile), StandardCharsets.UTF_8))) {
115116
String line;
116117
while ((line = br.readLine()) != null) {
117118
bw.write(template(line, execClass, crashFile));
118119
bw.newLine();
119120
}
120121
}
121-
Files.setPosixFilePermissions(scriptPath, fromString(R_XR_XR_X));
122+
scriptFile.setReadable(true, false);
123+
scriptFile.setWritable(false, false);
124+
scriptFile.setExecutable(true, false);
122125
}
123126
}
124127

‎dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package datadog.crashtracking;
22

33
import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY;
4-
import static java.util.Comparator.reverseOrder;
54
import static java.util.Locale.ROOT;
65

76
import com.datadoghq.profiler.JVMAccess;
@@ -12,25 +11,19 @@
1211
import datadog.libs.ddprof.DdprofLibraryLoader;
1312
import datadog.trace.api.Platform;
1413
import datadog.trace.util.TempLocationManager;
15-
import java.io.IOException;
14+
import java.io.File;
1615
import java.io.InputStream;
1716
import java.lang.management.ManagementFactory;
1817
import java.net.URL;
19-
import java.nio.file.Files;
20-
import java.nio.file.Path;
21-
import java.nio.file.Paths;
18+
import java.util.Arrays;
2219
import java.util.List;
2320
import java.util.StringTokenizer;
24-
import java.util.function.Predicate;
25-
import java.util.stream.Stream;
2621
import org.slf4j.Logger;
2722
import org.slf4j.LoggerFactory;
2823

2924
public final class Initializer {
3025
static final Logger LOG = LoggerFactory.getLogger(Initializer.class);
3126
static final String PID_PREFIX = "_pid";
32-
static final String RWXRWXRWX = "rwxrwxrwx";
33-
static final String R_XR_XR_X = "r-xr-xr-x";
3427

3528
private interface FlagAccess {
3629
String getValue(String flagName);
@@ -251,8 +244,8 @@ private static boolean isXdumpToolConfigured() {
251244
*/
252245
private static String getJ9CrashUploaderScriptPath() {
253246
String scriptFileName = getScriptFileName("dd_crash_uploader");
254-
Path scriptPath = TempLocationManager.getInstance().getTempDir().resolve(scriptFileName);
255-
return scriptPath.toString();
247+
String tempDir = TempLocationManager.getInstance().getTempDir().toString();
248+
return tempDir + File.separator + scriptFileName;
256249
}
257250

258251
static InputStream getCrashUploaderTemplate() {
@@ -280,19 +273,11 @@ static String findAgentJar() {
280273
else if (selfClass.startsWith("file:")) {
281274
int idx = selfClass.lastIndexOf("dd-java-agent");
282275
if (idx > -1) {
283-
Path libsPath = Paths.get(selfClass.substring(5, idx + 13), "build", "libs");
284-
try (Stream<Path> files = Files.walk(libsPath)) {
285-
Predicate<Path> isJarFile =
286-
p -> p.getFileName().toString().toLowerCase(ROOT).endsWith(".jar");
287-
agentPath =
288-
files
289-
.sorted(reverseOrder())
290-
.filter(isJarFile)
291-
.findFirst()
292-
.map(Path::toString)
293-
.orElse(null);
294-
} catch (IOException ignored) {
295-
// Ignore failure to get agent path
276+
File libsDir = new File(selfClass.substring(5, idx + 13), "build/libs");
277+
File[] jars = libsDir.listFiles(f -> f.getName().toLowerCase(ROOT).endsWith(".jar"));
278+
if (jars != null && jars.length > 0) {
279+
Arrays.sort(jars, (a, b) -> b.getName().compareTo(a.getName()));
280+
agentPath = jars[0].getAbsolutePath();
296281
}
297282
}
298283
}

0 commit comments

Comments
 (0)