Skip to content

Commit 2f9b45b

Browse files
ibauersachssrijeet0406
ibauersachs
and
srijeet0406
committed
Replace synchronization in Zone with locks
In addition to the locking change, the class is now also fully thread safe while iterating. The iterator additionally supports RRset removal now. JCStress is used to validate concurrency assumptions. Closes #306 Co-authored-by: Srijeet Chatterjee <srijeet0406@gmail.com>
1 parent e206796 commit 2f9b45b

File tree

10 files changed

+1454
-338
lines changed

10 files changed

+1454
-338
lines changed

‎.github/workflows/build.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,14 @@ jobs:
4545
- name: Build with Maven
4646
shell: bash
4747
run: |
48+
TEST_EXCLUSIONS=$([ '${{ matrix.java }}' == '21' ] && echo "" || echo "concurrency" )
4849
# don't exit immediately
4950
set +e
5051
mvn verify \
5152
-B \
5253
-Dsurefire.rerunFailingTestsCount=2 \
5354
-"Dgpg.skip" \
55+
-DexcludedGroups="${TEST_EXCLUSIONS}" \
5456
jacoco:report
5557
cd target
5658
mv jacoco.exec jacoco-${{ matrix.java }}-${{ matrix.arch }}-${{ matrix.os }}.exec

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@
66
target/
77
.DS_Store
88
.vscode/
9+
jcstress-results-*

‎checkstyle/checkstyle-suppressions.xml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?xml version="1.0"?>
2+
<!DOCTYPE suppressions PUBLIC
3+
"-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN"
4+
"https://checkstyle.org/dtds/suppressions_1_2.dtd">
5+
<suppressions>
6+
<!-- Exclude generated files in the output directory -->
7+
<suppress files="[/\\]target[/\\]" checks=".*"/>
8+
</suppressions>

‎pom.xml‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,17 @@
108108
<artifactId>lombok</artifactId>
109109
<version>${lombok.version}</version>
110110
</path>
111+
<path>
112+
<groupId>org.openjdk.jcstress</groupId>
113+
<artifactId>jcstress-core</artifactId>
114+
<version>0.16</version>
115+
<exclusions>
116+
<exclusion>
117+
<groupId>*</groupId>
118+
<artifactId>*</artifactId>
119+
</exclusion>
120+
</exclusions>
121+
</path>
111122
</annotationProcessorPaths>
112123
</configuration>
113124
</plugin>
@@ -334,6 +345,7 @@
334345
</includes>
335346
<excludes>
336347
<exclude>*.iml</exclude>
348+
<exclude>jcstress-*</exclude>
337349
</excludes>
338350
<trimTrailingWhitespace/>
339351
<endWithNewline/>
@@ -375,6 +387,7 @@
375387
<configLocation>google_checks.xml</configLocation>
376388
-->
377389
<configLocation>checkstyle/checkstyle-config.xml</configLocation>
390+
<suppressionsLocation>checkstyle/checkstyle-suppressions.xml</suppressionsLocation>
378391
<includeTestSourceDirectory>true</includeTestSourceDirectory>
379392
<linkXRef>false</linkXRef>
380393
<consoleOutput>true</consoleOutput>
@@ -399,6 +412,17 @@
399412
<groupId>org.apache.maven.plugins</groupId>
400413
<artifactId>maven-clean-plugin</artifactId>
401414
<version>3.3.2</version>
415+
<configuration>
416+
<filesets>
417+
<fileset>
418+
<!-- JCStress writes to the project directory without any option to configure the target -->
419+
<directory>${project.basedir}</directory>
420+
<includes>
421+
<include>jcstress-*</include>
422+
</includes>
423+
</fileset>
424+
</filesets>
425+
</configuration>
402426
</plugin>
403427

404428
<plugin>
@@ -583,6 +607,12 @@
583607
<version>2.16.0</version>
584608
<scope>test</scope>
585609
</dependency>
610+
<dependency>
611+
<groupId>org.openjdk.jcstress</groupId>
612+
<artifactId>jcstress-core</artifactId>
613+
<version>0.16</version>
614+
<scope>test</scope>
615+
</dependency>
586616
</dependencies>
587617

588618
<profiles>

‎src/main/java/org/xbill/DNS/RRset.java‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
* @author Brian Wellington
2121
*/
2222
@EqualsAndHashCode(of = {"rrs", "sigs"})
23-
public class RRset implements Serializable {
23+
public class RRset implements Serializable, Iterable<Record> {
2424
private final ArrayList<Record> rrs;
2525
private final ArrayList<RRSIGRecord> sigs;
2626
private short position;
@@ -51,6 +51,20 @@ public RRset(Record... records) {
5151
}
5252
}
5353

54+
/**
55+
* Creates an RRset and sets its contents to the specified record(s)
56+
*
57+
* @param records The records to add to the set. See {@link #addRR(Record)} for restrictions.
58+
* @since 3.6
59+
*/
60+
public RRset(Iterable<Record> records) {
61+
this();
62+
Objects.requireNonNull(records);
63+
for (Record r : records) {
64+
addRR(r);
65+
}
66+
}
67+
5468
/** Creates an RRset with the contents of an existing RRset */
5569
public RRset(RRset rrset) {
5670
rrs = new ArrayList<>(rrset.rrs);
@@ -191,11 +205,30 @@ public List<RRSIGRecord> sigs() {
191205
return Collections.unmodifiableList(sigs);
192206
}
193207

194-
/** Returns the number of (data) records */
208+
/** Returns the number of data records. */
195209
public int size() {
196210
return rrs.size();
197211
}
198212

213+
/**
214+
* Returns the number of signature records.
215+
*
216+
* @since 3.6
217+
*/
218+
public int sigSize() {
219+
return sigs.size();
220+
}
221+
222+
/**
223+
* Returns {@code true} if this RRset is empty, i.e. if there are neither data nor signature
224+
* records.
225+
*
226+
* @since 3.6
227+
*/
228+
public boolean isEmpty() {
229+
return rrs.isEmpty() && sigs.isEmpty();
230+
}
231+
199232
/**
200233
* Returns the name of the records
201234
*
@@ -289,4 +322,15 @@ public String toString() {
289322
sb.append(" }");
290323
return sb.toString();
291324
}
325+
326+
/**
327+
* Returns an {@link Iterator} over the resource records. This is a convenience method / interface
328+
* implementation and equivalent to calling {@code rrs().iterator()}.
329+
*
330+
* @since 3.6
331+
*/
332+
@Override
333+
public Iterator<Record> iterator() {
334+
return rrs().iterator();
335+
}
292336
}

‎src/main/java/org/xbill/DNS/SetResponseType.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,23 @@ enum SetResponseType {
2626
DELEGATION(true, false),
2727

2828
/**
29-
* The Cache/Zone found a CNAME when looking for the name.
29+
* The {@link Cache} or {@link Zone} found a CNAME when looking for the name.
3030
*
3131
* @see CNAMERecord
3232
*/
3333
CNAME(true, false),
3434

3535
/**
36-
* The Cache/Zone found a DNAME when looking for the name.
36+
* The {@link Cache} or {@link Zone} found a DNAME when looking for the name.
3737
*
3838
* @see DNAMERecord
3939
*/
4040
DNAME(true, false),
4141

42-
/** The Cache/Zone has successfully answered the question for the requested name/type/class. */
42+
/**
43+
* The {@link Cache} or {@link Zone} has successfully answered the question for the requested
44+
* name/type/class.
45+
*/
4346
SUCCESSFUL(false, false);
4447

4548
private final boolean printRecords;

0 commit comments

Comments
 (0)