RFR: 8238712: ConcurrentModificationException thrown when forward reference to record type in function prototype

[Maurizio Cimadamore]

On Tue, 11 Feb 2020 18:12:18 GMT, Henry Jen <henryjen at openjdk.org> wrote:

> CME get thrown when nested call of createTree introduce a new declaration into the map, this is too restrictive as demonstrated by the test case that it is likely to happen when forward reference of a struct, which will introduce a new declaration while processing the function declaration.
> 
> The fix allows nested call to add new declaration and only throw CME if replace actually happens.
> 
> Also included a minor fix that should abort generate function or could cause NPE.

Marked as reviewed by mcimadamore (Committer).

src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/TreeMaker.java line 58:

> 57:         // The supplier function may lead to adding some other type, which will cause CME using computeIfAbsent.
> 58:         // This implementation relax the constraint a bit only check for same key
> 59:         Declaration rv;

Nice workaround - basically only throw if we see that we're about to add something that was already there,

test/jdk/java/jextract/SmokeTest.java line 62:

> 61:     @Test
> 62:     public void test8238712() {
> 63:         Path header = Paths.get(System.getProperty("test.src.path", "."), "Test8238712.h");

At some point (e.g. separate changeset) it would be nice to split the tests from the framework here. E.g. SmokeTest has accumulated a lot of useful stuff which allow us to write API tests easily. But, currently, we are putting the tests together with the framework. We should make the framework a superclass, and then have a bunch of separate test subclasses - one per header.

src/jdk.incubator.jextract/share/classes/jdk/incubator/jextract/tool/StaticWrapperSourceFactory.java line 53:

> 52:             //abort
> 53:             return null;
> 54:         }

good catch!

-------------

PR: https://git.openjdk.java.net/panama-foreign/pull/16