Skip to content

Commit

Permalink
8318082: ConcurrentModificationException from IndexWriter
Browse files Browse the repository at this point in the history
Reviewed-by: jjg
  • Loading branch information
pavelrappo committed Oct 23, 2023
1 parent 729f4c5 commit fc29a2e
Show file tree
Hide file tree
Showing 9 changed files with 214 additions and 35 deletions.
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -113,6 +113,11 @@ public void buildPage() throws DocFileIOException {
getDescription(), body);
}

@Override
public boolean isIndexable() {
return true;
}

/**
* Adds the index to the documentation.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -769,4 +769,9 @@ protected Content getMemberDetails(Content content) {
}
return section;
}

@Override
public boolean isIndexable() {
return true;
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -301,6 +301,11 @@ private List<? extends DocTree> getLocalHeaderTags(List<? extends DocTree> dtree
}
return localTags;
}

@Override
public boolean isIndexable() {
return true;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,20 @@ public TagletWriter getTagletWriterInstance(TagletWriter.Context context) {
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.
*
Expand Down Expand Up @@ -1369,7 +1383,8 @@ public Boolean visitInheritDoc(InheritDocTree node, Content content) {
@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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -923,4 +923,9 @@ public void addPackageDeprecationInfo(Content li, PackageElement pkg) {
li.add(deprDiv);
}
}

@Override
public boolean isIndexable() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -455,4 +455,9 @@ private List<PackageElement> filterPackages(Predicate<? super PackageElement> fi
.filter(p -> p != packageElement && filter.test(p))
.collect(Collectors.toList());
}

@Override
public boolean isIndexable() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
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;
Expand Down Expand Up @@ -370,7 +372,8 @@ public Content createAnchorAndSearchIndex(Element element, String tagText, Strin
@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);
Expand Down
164 changes: 164 additions & 0 deletions test/langtools/jdk/javadoc/doclet/testIndex/TestSelfIndexing.java
Original file line number Diff line number Diff line change
@@ -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));
}
}
34 changes: 3 additions & 31 deletions test/langtools/jdk/javadoc/doclet/testSearch/TestSearch.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -503,14 +503,6 @@ constant in enum class pkg2.<a href="pkg2/TestEnum.html" title="enum class in p\
<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() {
Expand Down Expand Up @@ -621,13 +613,7 @@ ion interface pkg2.TestAnnotationType</dt>""",
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">\
Expand Down Expand Up @@ -665,13 +651,7 @@ ion interface pkg2.TestAnnotationType</dt>""",
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() {
Expand Down Expand Up @@ -839,20 +819,12 @@ void checkSearchTagIndex() {
{"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"}""",
"""
Expand Down

1 comment on commit fc29a2e

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.