8318082: ConcurrentModificationException from IndexWriter

Reviewed-by: jjg
This commit is contained in:
Pavel Rappo 2023-10-23 09:24:51 +00:00
parent 729f4c5d14
commit fc29a2e152
9 changed files with 214 additions and 35 deletions
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html
test/langtools/jdk/javadoc/doclet

@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -113,6 +113,11 @@ public abstract class AbstractOverviewIndexWriter extends HtmlDocletWriter {
getDescription(), body);
}
@Override
public boolean isIndexable() {
return true;
}
/**
* Adds the index to the documentation.
*

@ -769,4 +769,9 @@ public class ClassWriter extends SubWriterHolderWriter {
}
return section;
}
@Override
public boolean isIndexable() {
return true;
}
}

@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -301,6 +301,11 @@ public class DocFilesHandler {
}
return localTags;
}
@Override
public boolean isIndexable() {
return true;
}
}
}

@ -411,6 +411,20 @@ public abstract class HtmlDocletWriter {
return new TagletWriter(this, context);
}
/**
* {@return true if the page written by this writer should be indexed,
* false otherwise}
*
* Some pages merely aggregate filtered information available on other pages
* and, thus, have no indexing value. In fact, if indexed, they would
* clutter the index and mislead the reader.
*
* @implSpec The default implementation returns {@code false}.
*/
public boolean isIndexable() {
return false;
}
/**
* Generates the HTML document tree and prints it out.
*
@ -1369,7 +1383,8 @@ public abstract class HtmlDocletWriter {
@Override
public Boolean visitStartElement(StartElementTree node, Content content) {
Content attrs = new ContentBuilder();
if (node.getName().toString().matches("(?i)h[1-6]")) {
if (node.getName().toString().matches("(?i)h[1-6]")
&& isIndexable()) {
createSectionIdAndIndex(node, trees, attrs, element, context);
}
for (DocTree dt : node.getAttributes()) {

@ -923,4 +923,9 @@ public class ModuleWriter extends HtmlDocletWriter {
li.add(deprDiv);
}
}
@Override
public boolean isIndexable() {
return true;
}
}

@ -455,4 +455,9 @@ public class PackageWriter extends HtmlDocletWriter {
.filter(p -> p != packageElement && filter.test(p))
.collect(Collectors.toList());
}
@Override
public boolean isIndexable() {
return true;
}
}

@ -49,6 +49,8 @@ import jdk.javadoc.internal.doclets.formats.html.HtmlConfiguration;
import jdk.javadoc.internal.doclets.formats.html.HtmlDocletWriter;
import jdk.javadoc.internal.doclets.formats.html.HtmlIds;
import jdk.javadoc.internal.doclets.formats.html.HtmlOptions;
import jdk.javadoc.internal.doclets.formats.html.IndexWriter;
import jdk.javadoc.internal.doclets.formats.html.SummaryListWriter;
import jdk.javadoc.internal.doclets.formats.html.markup.ContentBuilder;
import jdk.javadoc.internal.doclets.formats.html.markup.HtmlId;
import jdk.javadoc.internal.doclets.formats.html.markup.HtmlStyle;
@ -370,7 +372,8 @@ public class TagletWriter {
@SuppressWarnings("preview")
Content createAnchorAndSearchIndex(Element element, String tagText, Content tagContent, String desc, DocTree tree) {
Content result;
if (context.isFirstSentence && context.inSummary || context.inTags.contains(DocTree.Kind.INDEX)) {
if (context.isFirstSentence && context.inSummary || context.inTags.contains(DocTree.Kind.INDEX)
|| !htmlWriter.isIndexable()) {
result = tagContent;
} else {
HtmlId id = HtmlIds.forText(tagText, htmlWriter.indexAnchorTable);

@ -0,0 +1,164 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Optional;
import java.util.Spliterator;
import java.util.Spliterators;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
import javadoc.tester.JavadocTester;
import toolbox.ToolBox;
/*
* @test
* @bug 8318082
* @library /tools/lib ../../lib
* @modules jdk.javadoc/jdk.javadoc.internal.tool
* @build toolbox.ToolBox javadoc.tester.*
* @run main TestSelfIndexing
*/
public class TestSelfIndexing extends JavadocTester {
public static void main(String... args) throws Exception {
new TestSelfIndexing().runTests();
}
private final ToolBox tb = new ToolBox();
/*
* Pages derived from other pages must not be indexed and may not
* cross-reference each other except for navigation ergonomics.
*
* For example, it's okay for all-index.html to reference deprecated-list.html;
* but it is not okay, for all-index.html to reference an anchor, such as
* deprecated-list.html#java.lang.Object.finalize()
*/
@Test
public void test(Path base) throws Exception {
Path src = base.resolve("src");
int i = 0;
// try to start a search tag (i) with the same letter, H,
// as the class, Hello, and (ii) with some other letter, P
for (var l : List.of("H", "P")) {
// try all markup constructs that cause indexing
for (var t : List.of("<h2>%s</h2>", "{@index %s}", "{@systemProperty %s}")) {
tb.writeJavaFiles(src, """
package pkg;
/** @deprecated %s */
public class Hello { }
""".formatted(t.formatted(l)));
Path out = base.resolve("out-" + i);
checking(t.formatted(l) + "; results in: " + out);
setAutomaticCheckNoStacktrace(true); // no exceptions
javadoc("-d", out.toString(),
"--source-path", src.toString(),
"pkg");
// check that index pages do not refer to derived pages
try (var s = findIndexFiles(out)) {
record PathAndString(Path path, String str) { }
Optional<PathAndString> r = s.map(p -> {
try {
return new PathAndString(p, Files.readString(p));
} catch (IOException e) {
throw new UncheckedIOException(e);
}
})
.flatMap(pac -> findLinksToDerivedPages(pac.str)
.map(link -> new PathAndString(pac.path, link)))
.findAny();
r.ifPresentOrElse(p -> failed(p.toString()), () -> passed(t.formatted(l)));
}
i++;
}
}
}
// ----------- support and infrastructure -----------
private static Stream<Path> findIndexFiles(Path start) throws IOException {
return Files.find(start, Integer.MAX_VALUE, (path, attr) -> {
if (attr.isDirectory())
return false;
var fileName = path.getFileName().toString();
if (!fileName.endsWith(".html") && !fileName.endsWith(".js"))
return false;
if (!fileName.contains("-index") && !fileName.contains("index-"))
return false;
var underDocFiles = StreamSupport.stream(Spliterators.spliterator(path.iterator(),
Integer.MAX_VALUE, Spliterator.ORDERED), false)
.anyMatch(p -> p.equals(DOC_FILES));
return !underDocFiles;
});
}
private static final Path DOC_FILES = Path.of("doc-files");
// good enough to capture relevant parts of URLs that javadoc uses,
// from html and js files alike
private static final Pattern URL = Pattern.compile(
"(?<path>([a-zA-Z.%0-9-]+/)*+)(?<file>[a-zA-Z.%0-9-]+\\.html)#[a-zA-Z.%0-9-]+");
static {
assert findLinksToDerivedPages("module-summary.html#a").findAny().isEmpty();
assert findLinksToDerivedPages("package-summary.html#a").findAny().isEmpty();
assert findLinksToDerivedPages("Exception.html#a").findAny().isEmpty();
assert findLinksToDerivedPages("util/doc-files/coll-index.html#a").findAny().isEmpty();
assert findLinksToDerivedPages("util/doc-files/index-all.html#a").findAny().isEmpty(); // tricky
assert findLinksToDerivedPages("index-all.html#a").findAny().isPresent();
assert findLinksToDerivedPages("index-17.html#a").findAny().isPresent();
}
// NOTE: this will not find self-links that are allowed on some index pages.
// For example, the quick-jump first-character links, such as #I:A,
// #I:B, etc., on the top and at the bottom of index-all.html
private static Stream<String> findLinksToDerivedPages(String content) {
return URL.matcher(content).results()
.filter(r -> {
String f = r.group("file");
if (!f.contains("-"))
return false;
return switch (f) {
case "package-summary.html",
"module-summary.html",
"overview-summary.html",
"help-doc.html" -> false;
default -> {
String p = r.group("path");
yield !p.contains("/doc-files/") && !p.startsWith("doc-files/");
}
};
})
.map(r -> r.group(0));
}
}

@ -26,7 +26,7 @@
* @bug 8141492 8071982 8141636 8147890 8166175 8168965 8176794 8175218 8147881
* 8181622 8182263 8074407 8187521 8198522 8182765 8199278 8196201 8196202
* 8184205 8214468 8222548 8223378 8234746 8241219 8254627 8247994 8263528
* 8266808 8248863 8305710
* 8266808 8248863 8305710 8318082
* @summary Test the search feature of javadoc.
* @library ../../lib
* @modules jdk.javadoc/jdk.javadoc.internal.tool
@ -503,14 +503,6 @@ public class TestSearch extends JavadocTester {
<dt><a href="pkg2/TestEnum.html#TWO" class="member-name-link">TWO</a> - Enum con\
stant in enum class pkg2.<a href="pkg2/TestEnum.html" title="enum class in pkg2"\
>TestEnum</a></dt>""");
checkOutput("index-all.html", true,
"""
<div class="deprecation-comment">class_test1 passes. Search tag <span id="Search\
TagDeprecatedClass" class="search-tag-result">SearchTagDeprecatedClass</span></d\
iv>""",
"""
<div class="deprecation-comment">error_test3 passes. Search tag for
method <span id="SearchTagDeprecatedMethod" class="search-tag-result">SearchTagDeprecatedMethod</span></div>""");
}
void checkSplitIndex() {
@ -621,13 +613,7 @@ public class TestSearch extends JavadocTester {
k">SearchTagDeprecatedClass</a> - Search tag in class pkg2.TestClass</dt>""",
"""
<dt><a href="pkg/package-summary.html#SingleWord" class="search-tag-link">Single\
Word</a> - Search tag in package pkg</dt>""",
"""
<div class="deprecation-comment">class_test1 passes. Search tag <span id="Search\
TagDeprecatedClass">SearchTagDeprecatedClass</div>""",
"""
<div class="deprecation-comment">error_test3 passes. Search tag for
method <span id="SearchTagDeprecatedMethod">SearchTagDeprecatedMethod</span></div>""");
Word</a> - Search tag in package pkg</dt>""");
checkOutput("index-all.html", true,
"""
<dt><a href="pkg2/TestEnum.html#searchphrasedeprecated" class="search-tag-link">\
@ -665,13 +651,7 @@ public class TestSearch extends JavadocTester {
search phrase deprecated</a> - Search tag in pkg2.TestEnum.ONE</dt>""",
"""
<dt><a href="pkg2/TestError.html#SearchTagDeprecatedMethod" class="search-tag-li\
nk">SearchTagDeprecatedMethod</a> - Search tag in pkg2.TestError.TestError()</dt>""",
"""
<div class="deprecation-comment">class_test1 passes. Search tag <span id="Search\
TagDeprecatedClass">SearchTagDeprecatedClass</span></div>""",
"""
<div class="deprecation-comment">error_test3 passes. Search tag for
method <span id="SearchTagDeprecatedMethod">SearchTagDeprecatedMethod</span></div>""");
nk">SearchTagDeprecatedMethod</a> - Search tag in pkg2.TestError.TestError()</dt>""");
}
void checkJavaFXOutput() {
@ -839,20 +819,12 @@ public class TestSearch extends JavadocTester {
{"l":"r","h":"package pkg","u":"pkg/package-summary.html#r"}""",
"""
{"l":"search phrase","h":"class pkg1.RegClass","d":"with description","u":"pkg1/RegClass.html#searchphrase"}""",
"""
{"l":"search phrase deprecated","h":"pkg2.TestEnum.ONE","u":"deprecated-list.html#searchphrasedeprecated"}""",
"""
{"l":"search phrase deprecated","h":"pkg2.TestEnum.ONE","u":"pkg2/TestEnum.html#searchphrasedeprecated"}""",
"""
{"l":"search phrase with desc deprecated","h":"annotation interface pkg2.TestAnnotationType","d":"description for phrase deprecated","u":"deprecated-list.html#searchphrasewithdescdeprecated"}""",
"""
{"l":"search phrase with desc deprecated","h":"annotation interface pkg2.TestAnnotationType","d":"description for phrase deprecated","u":"pkg2/TestAnnotationType.html#searchphrasewithdescdeprecated"}""",
"""
{"l":"SearchTagDeprecatedClass","h":"class pkg2.TestClass","u":"deprecated-list.html#SearchTagDeprecatedClass"}""",
"""
{"l":"SearchTagDeprecatedClass","h":"class pkg2.TestClass","u":"pkg2/TestClass.html#SearchTagDeprecatedClass"}""",
"""
{"l":"SearchTagDeprecatedMethod","h":"pkg2.TestError.TestError()","d":"with description","u":"deprecated-list.html#SearchTagDeprecatedMethod"}""",
"""
{"l":"SearchTagDeprecatedMethod","h":"pkg2.TestError.TestError()","d":"with description","u":"pkg2/TestError.html#SearchTagDeprecatedMethod"}""",
"""