Skip to content

Commit 0933194

Browse files
dougqhdougqh
authored
NativeLoader Listener Support (#9903)
* Adding listener support to NativeLoader * Adding listener tests - adding test to CompositeLibraryLoadingListener * isEmpty -> isNop to better convey purpose * Adding listener tests - adding test of NativeLoader.Builder listener related methods * Adding listener tests - adding end-to-end test for preloading listener support * spotless * More end-to-end listener tests * More end-to-end listener testing - checking platformSpec overrides * More end-to-end listener tests - checking component support * More end-to-end listener tests - added tests for temporary file creation - exposed a problem with TestLibraryLoadingListener errors getting swallowed in CompositeLibraryLoadingListener - adding failure tracking into TestLibraryLoadingListener that is checked in assertDone * spotless * Fixed errant text replacement in Javadoc * Adding clarifying comments * Tweaking listener API - passing underlying cause rather than LibraryLoadException - adding more implied assertions to TestLibraryLoadingListener - using LibCheck throughout TestLibraryLoadingListener * More JavaDoc & spotless * More end-to-end listener tests - adding temp file creation failure test * More end-to-end listener testing - checking temp file creation failure * spotless * Adding coverage - exposed oversight in CompositeLibraryLoadingListener not implementing onLoadFailure - added loading failure tests - exposed missing / incomplete load variants in NativeLoader * API clean-up Made order of PlatformSpec, component, libName consistent Changed component -> optionalComponent for clarity * API clean-up component -> optionalComponent * Fixing up oversights in previous parameter reordering * More test coverage Adding coverage for exceptions in LibraryResolver and PathLocator * More coverage - checking that proper exceptions are passed to listener - covering NativeLoader.isPlatformSupported * spotless * More coverage - removed unused parameter in onTempFileCreationFailure - fixed oversight in Composite listener coverage - intended to use a shuffled set of listeners with errant & nop listeners injected - also provides full coverage of default methods on LibraryLoadingListener * spotless
1 parent 640a4bd commit 0933194

19 files changed

+1507
-84
lines changed

‎components/native-loader/src/main/java/datadog/nativeloader/ClassLoaderResourcePathLocator.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ public ClassLoaderResourcePathLocator(final ClassLoader classLoader, final Strin
1414
}
1515

1616
@Override
17-
public URL locate(String component, String path) {
18-
return this.classLoader.getResource(PathUtils.concatPath(component, this.baseResource, path));
17+
public URL locate(String optionalComponent, String path) {
18+
return this.classLoader.getResource(
19+
PathUtils.concatPath(optionalComponent, this.baseResource, path));
1920
}
2021

2122
@Override

‎components/native-loader/src/main/java/datadog/nativeloader/CompositeLibraryLoadingListener.java‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
package datadog.nativeloader;
2+
3+
import java.net.URL;
4+
import java.nio.file.Path;
5+
import java.util.ArrayList;
6+
import java.util.Arrays;
7+
import java.util.Collection;
8+
9+
final class CompositeLibraryLoadingListener extends SafeLibraryLoadingListener {
10+
private final Collection<? extends LibraryLoadingListener> listeners;
11+
12+
CompositeLibraryLoadingListener(LibraryLoadingListener... listeners) {
13+
this(Arrays.asList(listeners));
14+
}
15+
16+
CompositeLibraryLoadingListener(Collection<? extends LibraryLoadingListener> listeners) {
17+
this.listeners = listeners;
18+
}
19+
20+
@Override
21+
public boolean isNop() {
22+
return this.listeners.isEmpty();
23+
}
24+
25+
int size() {
26+
return this.listeners.size();
27+
}
28+
29+
@Override
30+
public void onResolveDynamic(
31+
PlatformSpec platformSpec,
32+
String optionalComponent,
33+
String libName,
34+
boolean isPreloaded,
35+
URL optionalUrl) {
36+
for (LibraryLoadingListener listener : this.listeners) {
37+
try {
38+
listener.onResolveDynamic(
39+
platformSpec, optionalComponent, libName, isPreloaded, optionalUrl);
40+
} catch (Throwable ignored) {
41+
}
42+
}
43+
}
44+
45+
@Override
46+
public void onResolveDynamicFailure(
47+
PlatformSpec platformSpec,
48+
String optionalComponent,
49+
String libName,
50+
Throwable optionalCause) {
51+
for (LibraryLoadingListener listener : this.listeners) {
52+
try {
53+
listener.onResolveDynamicFailure(platformSpec, optionalComponent, libName, optionalCause);
54+
} catch (Throwable ignored) {
55+
}
56+
}
57+
}
58+
59+
@Override
60+
public void onLoad(
61+
PlatformSpec platformSpec,
62+
String optionalComponent,
63+
String libName,
64+
boolean isPreloaded,
65+
Path optionalLibPath) {
66+
for (LibraryLoadingListener listener : this.listeners) {
67+
try {
68+
listener.onLoad(platformSpec, optionalComponent, libName, isPreloaded, optionalLibPath);
69+
} catch (Throwable ignored) {
70+
}
71+
}
72+
}
73+
74+
@Override
75+
public void onLoadFailure(
76+
PlatformSpec platformSpec,
77+
String optionalComponent,
78+
String libName,
79+
Throwable optionalCause) {
80+
for (LibraryLoadingListener listener : this.listeners) {
81+
try {
82+
listener.onLoadFailure(platformSpec, optionalComponent, libName, optionalCause);
83+
} catch (Throwable ignored) {
84+
}
85+
}
86+
}
87+
88+
@Override
89+
public void onTempFileCreated(
90+
PlatformSpec platformSpec, String optionalComponent, String libName, Path tempFile) {
91+
for (LibraryLoadingListener listener : this.listeners) {
92+
try {
93+
listener.onTempFileCreated(platformSpec, optionalComponent, libName, tempFile);
94+
} catch (Throwable ignored) {
95+
}
96+
}
97+
}
98+
99+
@Override
100+
public void onTempFileCreationFailure(
101+
PlatformSpec platformSpec,
102+
String optionalComponent,
103+
String libName,
104+
Path tempDir,
105+
String libExt,
106+
Throwable optionalCause) {
107+
for (LibraryLoadingListener listener : this.listeners) {
108+
try {
109+
listener.onTempFileCreationFailure(
110+
platformSpec, optionalComponent, libName, tempDir, libExt, optionalCause);
111+
} catch (Throwable ignored) {
112+
}
113+
}
114+
}
115+
116+
@Override
117+
public void onTempFileCleanup(
118+
PlatformSpec platformSpec, String optionalComponent, String libName, Path tempPath) {
119+
for (LibraryLoadingListener listener : this.listeners) {
120+
try {
121+
listener.onTempFileCleanup(platformSpec, optionalComponent, libName, tempPath);
122+
} catch (Throwable ignored) {
123+
}
124+
}
125+
}
126+
127+
@Override
128+
public CompositeLibraryLoadingListener join(LibraryLoadingListener... listeners) {
129+
ArrayList<LibraryLoadingListener> combinedListeners =
130+
new ArrayList<>(this.listeners.size() + listeners.length);
131+
combinedListeners.addAll(this.listeners);
132+
combinedListeners.addAll(Arrays.asList(listeners));
133+
return new CompositeLibraryLoadingListener(combinedListeners);
134+
}
135+
136+
@Override
137+
public String toString() {
138+
return this.getClass().getSimpleName() + ":" + this.listeners.toString();
139+
}
140+
}

‎components/native-loader/src/main/java/datadog/nativeloader/FlatDirLibraryResolver.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ private FlatDirLibraryResolver() {}
1313

1414
@Override
1515
public final URL resolve(
16-
PathLocator pathLocator, String component, PlatformSpec platformSpec, String libName)
16+
PathLocator pathLocator, PlatformSpec platformSpec, String optionalComponent, String libName)
1717
throws Exception {
1818
PathLocatorHelper pathLocatorHelper = new PathLocatorHelper(libName, pathLocator);
1919

@@ -28,22 +28,22 @@ public final URL resolve(
2828

2929
if (libcPath != null) {
3030
String specializedPath = regularPath + "-" + libcPath;
31-
url = pathLocatorHelper.locate(component, specializedPath + "/" + libFileName);
31+
url = pathLocatorHelper.locate(optionalComponent, specializedPath + "/" + libFileName);
3232
if (url != null) return url;
3333
}
3434

35-
url = pathLocatorHelper.locate(component, regularPath + "/" + libFileName);
35+
url = pathLocatorHelper.locate(optionalComponent, regularPath + "/" + libFileName);
3636
if (url != null) return url;
3737

38-
url = pathLocatorHelper.locate(component, osPath + "/" + libFileName);
38+
url = pathLocatorHelper.locate(optionalComponent, osPath + "/" + libFileName);
3939
if (url != null) return url;
4040

4141
// fallback to searching at top-level, mostly concession to good out-of-box behavior
4242
// with java.library.path
43-
url = pathLocatorHelper.locate(component, libFileName);
43+
url = pathLocatorHelper.locate(optionalComponent, libFileName);
4444
if (url != null) return url;
4545

46-
if (component != null) {
46+
if (optionalComponent != null) {
4747
url = pathLocatorHelper.locate(null, libFileName);
4848
if (url != null) return url;
4949
}

‎components/native-loader/src/main/java/datadog/nativeloader/LibDirBasedPathLocator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ public LibDirBasedPathLocator(File... libDirs) {
1414
}
1515

1616
@Override
17-
public URL locate(String component, String path) {
18-
String fullPath = PathUtils.concatPath(component, path);
17+
public URL locate(String optionalComponent, String path) {
18+
String fullPath = PathUtils.concatPath(optionalComponent, path);
1919

2020
for (File libDir : this.libDirs) {
2121
File libFile = new File(libDir, fullPath);

‎components/native-loader/src/main/java/datadog/nativeloader/LibFile.java‎

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,66 +16,114 @@ public final class LibFile implements AutoCloseable {
1616
static final boolean NO_CLEAN_UP = false;
1717
static final boolean CLEAN_UP = true;
1818

19-
static final LibFile preloaded(String libName) {
20-
return new LibFile(libName, null, NO_CLEAN_UP);
19+
static final LibFile preloaded(
20+
PlatformSpec platformSpec,
21+
String optionalComponent,
22+
String libName,
23+
SafeLibraryLoadingListener listeners) {
24+
return new LibFile(platformSpec, optionalComponent, libName, null, NO_CLEAN_UP, listeners);
2125
}
2226

23-
static final LibFile fromFile(String libName, File file) {
24-
return new LibFile(libName, file, NO_CLEAN_UP);
27+
static final LibFile fromFile(
28+
PlatformSpec platformSpec,
29+
String optionalComponent,
30+
String libName,
31+
File optionalFile,
32+
SafeLibraryLoadingListener listeners) {
33+
return new LibFile(
34+
platformSpec, optionalComponent, libName, optionalFile, NO_CLEAN_UP, listeners);
2535
}
2636

27-
static final LibFile fromTempFile(String libName, File file) {
28-
return new LibFile(libName, file, CLEAN_UP);
37+
static final LibFile fromTempFile(
38+
PlatformSpec platformSpec,
39+
String optionalComponent,
40+
String libName,
41+
File optionalFile,
42+
SafeLibraryLoadingListener listeners) {
43+
return new LibFile(platformSpec, optionalComponent, libName, optionalFile, CLEAN_UP, listeners);
2944
}
3045

46+
final PlatformSpec platformSpec;
47+
final String optionalComponent;
3148
final String libName;
3249

33-
final File file;
50+
final File optionalFile;
3451
final boolean needsCleanup;
3552

36-
LibFile(String libName, File file, boolean needsCleanup) {
53+
final SafeLibraryLoadingListener listeners;
54+
55+
LibFile(
56+
PlatformSpec platformSpec,
57+
String optionalComponent,
58+
String libName,
59+
File optionalFile,
60+
boolean needsCleanup,
61+
SafeLibraryLoadingListener listeners) {
62+
this.platformSpec = platformSpec;
63+
this.optionalComponent = optionalComponent;
3764
this.libName = libName;
3865

39-
this.file = file;
66+
this.optionalFile = optionalFile;
4067
this.needsCleanup = needsCleanup;
68+
69+
this.listeners = listeners;
4170
}
4271

4372
/** Indicates if this library was "preloaded" */
4473
public boolean isPreloaded() {
45-
return (this.file == null);
74+
return (this.optionalFile == null);
4675
}
4776

4877
/** Loads the underlying library into the JVM */
4978
public void load() throws LibraryLoadException {
50-
if (this.isPreloaded()) return;
79+
boolean isPreloaded = this.isPreloaded();
80+
if (isPreloaded) {
81+
this.listeners.onLoad(
82+
this.platformSpec, this.optionalComponent, this.libName, isPreloaded, null);
83+
return;
84+
}
5185

5286
try {
5387
Runtime.getRuntime().load(this.getAbsolutePath());
88+
89+
if (true) throw new RuntimeException("real load - worked?");
5490
} catch (Throwable t) {
91+
this.listeners.onLoadFailure(this.platformSpec, this.optionalComponent, this.libName, t);
5592
throw new LibraryLoadException(this.libName, t);
5693
}
94+
95+
this.listeners.onLoad(
96+
this.platformSpec,
97+
this.optionalComponent,
98+
this.libName,
99+
isPreloaded,
100+
this.optionalFile.toPath());
57101
}
58102

59103
/** Provides a File to the library -- returns null for pre-loaded libraries */
60104
public final File toFile() {
61-
return this.file;
105+
return this.optionalFile;
62106
}
63107

64108
/** Provides a Path to the library -- return null for pre-loaded libraries */
65109
public final Path toPath() {
66-
return this.file == null ? null : this.file.toPath();
110+
return this.optionalFile == null ? null : this.optionalFile.toPath();
67111
}
68112

69113
/** Provides the an absolute path to the library -- returns null for pre-loaded libraries */
70114
public final String getAbsolutePath() {
71-
return this.file == null ? null : this.file.getAbsolutePath();
115+
return this.optionalFile == null ? null : this.optionalFile.getAbsolutePath();
72116
}
73117

74-
/** Schedules clean-up of underlying file -- if the file is a temp file */
118+
/** Schedules clean-up of underlying optionalFile -- if the file is a temp file */
75119
@Override
76120
public void close() {
77121
if (this.needsCleanup) {
78-
NativeLoader.delete(this.file);
122+
boolean deleted = NativeLoader.delete(this.optionalFile);
123+
if (deleted) {
124+
this.listeners.onTempFileCleanup(
125+
this.platformSpec, this.optionalComponent, this.libName, this.optionalFile.toPath());
126+
}
79127
}
80128
}
81129
}

0 commit comments

Comments
 (0)