Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion odfdom/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<useModulePath>false</useModulePath>
<argLine>-javaagent:${settings.localRepository}/org/jacoco/org.jacoco.agent/${jacoco.version}/org.jacoco.agent-${jacoco.version}-runtime.jar=destfile=${project.build.directory}/coverage-reports/jacoco-ut.exec</argLine>
<argLine>-javaagent:${settings.localRepository}/org/jacoco/org.jacoco.agent/${jacoco.version}/org.jacoco.agent-${jacoco.version}-runtime.jar=destfile=${project.build.directory}/coverage-reports/jacoco-ut.exec,excludes=${jacoco.agent.excludes}</argLine>
<systemPropertyVariables>
<org.odftoolkit.odfdom.validation>true</org.odftoolkit.odfdom.validation>
</systemPropertyVariables>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
*/
package org.odftoolkit.odfdom.dom.style.props;

import java.util.Iterator;
import java.util.TreeSet;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.odftoolkit.odfdom.pkg.OdfName;

/**
Expand All @@ -33,15 +33,18 @@
*/
public class OdfStyleProperty implements Comparable<OdfStyleProperty> {

private OdfStylePropertiesSet m_propSet;
private OdfName m_name;
private final OdfStylePropertiesSet m_propSet;
private final OdfName m_name;

private OdfStyleProperty(OdfStylePropertiesSet propSet, OdfName name) {
m_propSet = propSet;
m_name = name;
}

private static TreeSet<OdfStyleProperty> m_styleProperties = new TreeSet<>();
// Concurrent map, as the static initializers of the generated Style*PropertiesElement classes
// might register their properties from different threads at the same time (see issue #442)
private static final Map<OdfStyleProperty, OdfStyleProperty> m_styleProperties =
new ConcurrentHashMap<>();

/**
* Looks if an OdfStyleProperty is already listed in the static sytleProperties set, otherwise
Expand All @@ -53,26 +56,8 @@ private OdfStyleProperty(OdfStylePropertiesSet propSet, OdfName name) {
*/
public static OdfStyleProperty get(OdfStylePropertiesSet propSet, OdfName name) {
OdfStyleProperty temp = new OdfStyleProperty(propSet, name);
// Replacement for (JDK1.6)
// OdfStyleProperty result = m_styleProperties.floor(temp);

Iterator<OdfStyleProperty> iter = m_styleProperties.iterator();
OdfStyleProperty result = null;

// check if key exists
if (!m_styleProperties.contains(temp)) {
m_styleProperties.add(temp);
return temp;
}
while (iter.hasNext()) {
result = iter.next();
if (result.equals(temp)) {
return result;
}
}

m_styleProperties.add(temp);
return temp;
OdfStyleProperty existing = m_styleProperties.putIfAbsent(temp, temp);
return existing != null ? existing : temp;
}

/** @return an OdfStylePropertiesSet member */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
/**
* **********************************************************************
*
* <p>DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER
*
* <p>Copyright 2026 The Document Foundation.
*
* <p>Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file
* except in compliance with the License. You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0. You can also obtain a copy of the License at
* http://odftoolkit.org/docs/license.txt
*
* <p>Unless required by applicable law or agreed to in writing, software distributed under the
* License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied.
*
* <p>See the License for the specific language governing permissions and limitations under the
* License.
*
* <p>**********************************************************************
*/
package org.odftoolkit.odfdom.dom.style.props;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import org.junit.Test;
import org.odftoolkit.odfdom.dom.OdfDocumentNamespace;
import org.odftoolkit.odfdom.pkg.OdfName;

/**
* Regression tests for concurrent registration of style properties, see
* https://github.com/tdf/odftoolkit/issues/442
*/
public class OdfStylePropertyConcurrencyTest {

private static final int THREADS = 32;
private static final int OPERATIONS_PER_THREAD = 5_000;
private static final long TIMEOUT_SECONDS = 60;

private static final String[] STYLE_PROPERTIES_ELEMENTS = {
"StyleChartPropertiesElement",
"StyleDrawingPagePropertiesElement",
"StyleGraphicPropertiesElement",
"StyleHeaderFooterPropertiesElement",
"StyleListLevelPropertiesElement",
"StylePageLayoutPropertiesElement",
"StyleParagraphPropertiesElement",
"StyleRubyPropertiesElement",
"StyleSectionPropertiesElement",
"StyleTableCellPropertiesElement",
"StyleTableColumnPropertiesElement",
"StyleTablePropertiesElement",
"StyleTableRowPropertiesElement",
"StyleTextPropertiesElement"
};

/** Many threads register different new properties at the same time. */
@Test
public void testConcurrentRegistration() throws Exception {
OdfStylePropertiesSet[] propSets = OdfStylePropertiesSet.values();
List<List<OdfStyleProperty>> results =
runConcurrently(
worker -> {
List<OdfStyleProperty> props = new ArrayList<>(OPERATIONS_PER_THREAD);
for (int i = 0; i < OPERATIONS_PER_THREAD; i++) {
OdfStylePropertiesSet propSet = propSets[(worker + i) % propSets.length];
OdfName name =
OdfName.newName(
OdfDocumentNamespace.TEXT, "race-unique-property-" + worker + "-" + i);
OdfStyleProperty prop = OdfStyleProperty.get(propSet, name);
assertSame(propSet, prop.getPropertySet());
assertSame(name, prop.getName());
props.add(prop);
}
return props;
});
// all properties have been registered and are found again
for (List<OdfStyleProperty> props : results) {
for (OdfStyleProperty prop : props) {
assertSame(prop, OdfStyleProperty.get(prop.getPropertySet(), prop.getName()));
}
}
}

/** Many threads register the same new properties at the same time and share one instance. */
@Test
public void testConcurrentCanonicalization() throws Exception {
List<List<OdfStyleProperty>> results =
runConcurrently(
worker -> {
List<OdfStyleProperty> props = new ArrayList<>(OPERATIONS_PER_THREAD);
for (int i = 0; i < OPERATIONS_PER_THREAD; i++) {
props.add(
OdfStyleProperty.get(
OdfStylePropertiesSet.TextProperties,
OdfName.newName(OdfDocumentNamespace.TEXT, "race-shared-property-" + i)));
}
return props;
});
List<OdfStyleProperty> expected = results.get(0);
for (List<OdfStyleProperty> props : results) {
assertEquals(OPERATIONS_PER_THREAD, props.size());
for (int i = 0; i < OPERATIONS_PER_THREAD; i++) {
assertSame(expected.get(i), props.get(i));
}
}
}

/**
* Initializes the generated Style*PropertiesElement classes concurrently, as their static
* initializers register their properties. This is only a real cold start, if no earlier test of
* the same JVM has already loaded these classes.
*/
@Test
public void testConcurrentStylePropertiesElementInitialization() throws Exception {
ClassLoader loader = getClass().getClassLoader();
List<List<Class<?>>> results =
runConcurrently(
worker -> {
List<Class<?>> classes = new ArrayList<>(STYLE_PROPERTIES_ELEMENTS.length);
for (int i = 0; i < STYLE_PROPERTIES_ELEMENTS.length; i++) {
String className =
"org.odftoolkit.odfdom.dom.element.style."
+ STYLE_PROPERTIES_ELEMENTS[(worker + i) % STYLE_PROPERTIES_ELEMENTS.length];
classes.add(Class.forName(className, true, loader));
}
return classes;
});
for (List<Class<?>> classes : results) {
assertEquals(STYLE_PROPERTIES_ELEMENTS.length, classes.size());
}
}

private interface Worker<T> {
T run(int worker) throws Exception;
}

/** Runs the given work in THREADS threads, all started at the same moment. */
private static <T> List<T> runConcurrently(Worker<T> work) throws Exception {
ExecutorService pool = Executors.newFixedThreadPool(THREADS);
try {
CountDownLatch ready = new CountDownLatch(THREADS);
CountDownLatch start = new CountDownLatch(1);
List<Future<T>> futures = new ArrayList<>(THREADS);
for (int thread = 0; thread < THREADS; thread++) {
final int worker = thread;
Callable<T> task =
() -> {
ready.countDown();
start.await();
return work.run(worker);
};
futures.add(pool.submit(task));
}
assertTrue(
"Worker threads did not start in time", ready.await(TIMEOUT_SECONDS, TimeUnit.SECONDS));
start.countDown();

List<T> results = new ArrayList<>(THREADS);
for (Future<T> future : futures) {
// rethrows any failure of a worker thread as ExecutionException
results.add(future.get(TIMEOUT_SECONDS, TimeUnit.SECONDS));
}
return results;
} finally {
pool.shutdownNow();
}
}
}
11 changes: 10 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@
<maven.javadoc.failOnError>false</maven.javadoc.failOnError>
<maven.build.timestamp.format>yyyy-MM-dd'T'HH:mm:ss</maven.build.timestamp.format>
<!-- JaCoCo Properties -->
<jacoco.version>0.8.14</jacoco.version>
<jacoco.version>0.8.15</jacoco.version>
<!-- JDK classes are never instrumented by the JaCoCo agent, as newer JDKs might use class file
versions unknown to JaCoCo resulting into exceptions in the build log -->
<jacoco.agent.excludes>java.*:javax.*:jdk.*:sun.*:com.sun.*</jacoco.agent.excludes>
<sonar.java.coveragePlugin>jacoco</sonar.java.coveragePlugin>
<sonar.dynamicAnalysis>reuseReports</sonar.dynamicAnalysis>
<sonar.coverage.jacoco.xmlReportPaths>${project.reporting.outputDirectory}/jacoco-ut/jacoco.xml</sonar.coverage.jacoco.xmlReportPaths>
Expand Down Expand Up @@ -494,6 +497,9 @@
<configuration>
<!-- Sets the path to the file which contains the execution data. -->
<destFile>${project.build.directory}/coverage-reports/jacoco-ut.exec</destFile>
<excludes>
<exclude>${jacoco.agent.excludes}</exclude>
</excludes>
<!--
Sets the name of the property containing the settings
for JaCoCo runtime agent.
Expand Down Expand Up @@ -535,6 +541,9 @@
<configuration>
<!-- Sets the path to the file which contains the execution data. -->
<destFile>${project.build.directory}/coverage-reports/jacoco-it.exec</destFile>
<excludes>
<exclude>${jacoco.agent.excludes}</exclude>
</excludes>
<!--
Sets the name of the property containing the settings
for JaCoCo runtime agent.
Expand Down
Loading