Skip to content

Commit 5609998

Browse files
ibauersachsibauersachs
committed
Reduce lock contention and disk access in hosts file parsing
Closes #369
1 parent 2c5a352 commit 5609998

File tree

2 files changed

+87
-21
lines changed

2 files changed

+87
-21
lines changed

‎src/main/java/org/xbill/DNS/hosts/HostsFileParser.java‎

Lines changed: 71 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@
88
import java.nio.file.Files;
99
import java.nio.file.Path;
1010
import java.nio.file.Paths;
11+
import java.time.Clock;
12+
import java.time.Duration;
1113
import java.time.Instant;
1214
import java.util.Arrays;
13-
import java.util.HashMap;
1415
import java.util.Map;
1516
import java.util.Objects;
1617
import java.util.Optional;
18+
import java.util.concurrent.ConcurrentHashMap;
1719
import java.util.concurrent.atomic.AtomicInteger;
1820
import lombok.RequiredArgsConstructor;
1921
import lombok.extern.slf4j.Slf4j;
@@ -32,13 +34,23 @@
3234
public final class HostsFileParser {
3335
private final int maxFullCacheFileSizeBytes =
3436
Integer.parseInt(System.getProperty("dnsjava.hostsfile.max_size_bytes", "16384"));
37+
private final Duration fileChangeCheckInterval =
38+
Duration.ofMillis(
39+
Integer.parseInt(
40+
System.getProperty("dnsjava.hostsfile.change_check_interval_ms", "300000")));
3541

36-
private final Map<String, InetAddress> hostsCache = new HashMap<>();
3742
private final Path path;
3843
private final boolean clearCacheOnChange;
44+
private Clock clock = Clock.systemUTC();
45+
46+
@SuppressWarnings("java:S3077")
47+
private volatile Map<String, InetAddress> hostsCache;
48+
49+
private Instant lastFileModificationCheckTime = Instant.MIN;
3950
private Instant lastFileReadTime = Instant.MIN;
4051
private boolean isEntireFileParsed;
4152
private boolean hostsFileWarningLogged = false;
53+
private long hostsFileSizeBytes;
4254

4355
/**
4456
* Creates a new instance based on the current OS's default. Unix and alike (or rather everything
@@ -88,8 +100,7 @@ public HostsFileParser(Path path, boolean clearCacheOnChange) {
88100
* @throws IllegalArgumentException when {@code type} is not {@link org.xbill.DNS.Type#A} or{@link
89101
* org.xbill.DNS.Type#AAAA}.
90102
*/
91-
public synchronized Optional<InetAddress> getAddressForHost(Name name, int type)
92-
throws IOException {
103+
public Optional<InetAddress> getAddressForHost(Name name, int type) throws IOException {
93104
Objects.requireNonNull(name, "name is required");
94105
if (type != Type.A && type != Type.AAAA) {
95106
throw new IllegalArgumentException("type can only be A or AAAA");
@@ -102,13 +113,11 @@ public synchronized Optional<InetAddress> getAddressForHost(Name name, int type)
102113
return Optional.of(cachedAddress);
103114
}
104115

105-
if (isEntireFileParsed || !Files.exists(path)) {
116+
if (isEntireFileParsed) {
106117
return Optional.empty();
107118
}
108119

109-
if (Files.size(path) <= maxFullCacheFileSizeBytes) {
110-
parseEntireHostsFile();
111-
} else {
120+
if (hostsFileSizeBytes > maxFullCacheFileSizeBytes) {
112121
searchHostsFileForEntry(name, type);
113122
}
114123

@@ -141,8 +150,6 @@ private void parseEntireHostsFile() throws IOException {
141150
nameFailures);
142151
hostsFileWarningLogged = true;
143152
}
144-
145-
isEntireFileParsed = true;
146153
}
147154

148155
private void searchHostsFileForEntry(Name name, int type) throws IOException {
@@ -235,21 +242,61 @@ private String[] getLineTokens(String line) {
235242
}
236243

237244
private void validateCache() throws IOException {
238-
if (clearCacheOnChange) {
245+
if (!clearCacheOnChange) {
246+
if (hostsCache == null) {
247+
synchronized (this) {
248+
if (hostsCache == null) {
249+
readHostsFile();
250+
}
251+
}
252+
}
253+
254+
return;
255+
}
256+
257+
if (lastFileModificationCheckTime.plus(fileChangeCheckInterval).isBefore(clock.instant())) {
258+
log.debug("Checked for changes more than 5minutes ago, checking");
239259
// A filewatcher / inotify etc. would be nicer, but doesn't work. c.f. the write up at
240260
// https://blog.arkey.fr/2019/09/13/watchservice-and-bind-mount/
241-
Instant fileTime =
242-
Files.exists(path) ? Files.getLastModifiedTime(path).toInstant() : Instant.MAX;
243-
if (fileTime.isAfter(lastFileReadTime)) {
244-
// skip logging noise when the cache is empty anyway
245-
if (!hostsCache.isEmpty()) {
246-
log.info("Local hosts database has changed at {}, clearing cache", fileTime);
247-
hostsCache.clear();
261+
262+
synchronized (this) {
263+
if (!lastFileModificationCheckTime
264+
.plus(fileChangeCheckInterval)
265+
.isBefore(clock.instant())) {
266+
log.debug("Never mind, check fulfilled in another thread");
267+
return;
268+
}
269+
270+
lastFileModificationCheckTime = clock.instant();
271+
readHostsFile();
272+
}
273+
}
274+
}
275+
276+
private void readHostsFile() throws IOException {
277+
if (Files.exists(path)) {
278+
Instant fileTime = Files.getLastModifiedTime(path).toInstant();
279+
if (!lastFileReadTime.equals(fileTime)) {
280+
createOrClearCache();
281+
282+
hostsFileSizeBytes = Files.size(path);
283+
if (hostsFileSizeBytes <= maxFullCacheFileSizeBytes) {
284+
parseEntireHostsFile();
285+
isEntireFileParsed = true;
248286
}
249287

250-
isEntireFileParsed = false;
251288
lastFileReadTime = fileTime;
252289
}
290+
} else {
291+
createOrClearCache();
292+
}
293+
}
294+
295+
private void createOrClearCache() {
296+
if (hostsCache == null) {
297+
hostsCache = new ConcurrentHashMap<>();
298+
} else {
299+
hostsCache.clear();
253300
}
254301
}
255302

@@ -259,6 +306,10 @@ private String key(Name name, int type) {
259306

260307
// for unit testing only
261308
int cacheSize() {
262-
return hostsCache.size();
309+
return hostsCache == null ? 0 : hostsCache.size();
310+
}
311+
312+
void setClock(Clock clock) {
313+
this.clock = clock;
263314
}
264315
}

‎src/test/java/org/xbill/DNS/hosts/HostsFileParserTest.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
import static org.junit.jupiter.api.Assertions.assertNotEquals;
66
import static org.junit.jupiter.api.Assertions.assertThrows;
77
import static org.junit.jupiter.api.Assertions.assertTrue;
8+
import static org.mockito.Mockito.mock;
9+
import static org.mockito.Mockito.when;
810

911
import java.io.BufferedWriter;
1012
import java.io.IOException;
@@ -18,6 +20,9 @@
1820
import java.nio.file.StandardCopyOption;
1921
import java.nio.file.StandardOpenOption;
2022
import java.nio.file.attribute.FileTime;
23+
import java.time.Clock;
24+
import java.time.Duration;
25+
import java.time.Instant;
2126
import java.util.Optional;
2227
import org.junit.jupiter.api.BeforeAll;
2328
import org.junit.jupiter.api.Test;
@@ -72,7 +77,7 @@ void testMissingFileIsEmptyResult() throws IOException {
7277
}
7378

7479
@Test
75-
void testCacheLookup() throws IOException {
80+
void testCacheLookupAfterFileDeleteWithoutChangeChecking() throws IOException {
7681
Path tempHosts = Files.copy(hostsFileWindows, tempDir, StandardCopyOption.REPLACE_EXISTING);
7782
HostsFileParser hostsFileParser = new HostsFileParser(tempHosts, false);
7883
assertEquals(0, hostsFileParser.cacheSize());
@@ -98,6 +103,10 @@ void testFileDeletionClearsCache() throws IOException {
98103
tempDir.resolve("testFileWatcherClearsCache"),
99104
StandardCopyOption.REPLACE_EXISTING);
100105
HostsFileParser hostsFileParser = new HostsFileParser(tempHosts);
106+
Clock clock = mock(Clock.class);
107+
hostsFileParser.setClock(clock);
108+
Instant now = Clock.systemUTC().instant();
109+
when(clock.instant()).thenReturn(now);
101110
assertEquals(0, hostsFileParser.cacheSize());
102111
assertEquals(
103112
kubernetesAddress,
@@ -106,6 +115,7 @@ void testFileDeletionClearsCache() throws IOException {
106115
.orElseThrow(() -> new IllegalStateException("Host entry not found")));
107116
assertTrue(hostsFileParser.cacheSize() > 1, "Cache must not be empty");
108117
Files.delete(tempHosts);
118+
when(clock.instant()).thenReturn(now.plus(Duration.ofMinutes(6)));
109119
assertEquals(Optional.empty(), hostsFileParser.getAddressForHost(kubernetesName, Type.A));
110120
assertEquals(0, hostsFileParser.cacheSize());
111121
}
@@ -119,6 +129,10 @@ void testFileChangeClearsCache() throws IOException {
119129
StandardCopyOption.REPLACE_EXISTING);
120130
Files.setLastModifiedTime(tempHosts, FileTime.fromMillis(0));
121131
HostsFileParser hostsFileParser = new HostsFileParser(tempHosts);
132+
Clock clock = mock(Clock.class);
133+
hostsFileParser.setClock(clock);
134+
Instant now = Clock.systemUTC().instant();
135+
when(clock.instant()).thenReturn(now);
122136
assertEquals(0, hostsFileParser.cacheSize());
123137
assertEquals(
124138
kubernetesAddress,
@@ -134,6 +148,7 @@ void testFileChangeClearsCache() throws IOException {
134148
}
135149

136150
Files.setLastModifiedTime(tempHosts, FileTime.fromMillis(10_0000));
151+
when(clock.instant()).thenReturn(now.plus(Duration.ofMinutes(6)));
137152
assertEquals(
138153
InetAddress.getByAddress(testName.toString(), localhostBytes),
139154
hostsFileParser

0 commit comments

Comments
 (0)