RFR: JDK-8189376 Unsorted $(wildcard) causes instable module-deps.gmk

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: JDK-8189376 Unsorted $(wildcard) causes instable module-deps.gmk

Magnus Ihse Bursie
We have implicitly assumed that subsequent calls to $(wildcard) will
return the same (sorted) order of files during a single build operation.
This is not guaranteed since GNU make 3.82, but will be the case in
practice on most systems due the the filesystem being used.

It turned out that if the order was different, FindAllModuleInfos
returned different strings which in turn caused module-deps.gmk to be
re-generated at any time during the build, causing race conditions.

The bug analysis and patch is contributed by Martin Buchholz.

Bug: https://bugs.openjdk.java.net/browse/JDK-8189376
Patch inline:
diff --git a/make/common/Modules.gmk b/make/common/Modules.gmk
--- a/make/common/Modules.gmk
+++ b/make/common/Modules.gmk
@@ -249,10 +249,10 @@
  # configuration.
  # Param 1 - Module to find for, set to * for finding all
  FindAllModuleInfos = \
-    $(wildcard \
+    $(sort $(wildcard \
          $(foreach sub, $(SRC_SUBDIRS), \
            $(patsubst %,%/$(strip $1)/$(sub)/module-info.java,
$(TOP_SRC_DIRS))) \
-        $(patsubst %,%/$(strip $1)/module-info.java,
$(IMPORT_MODULES_SRC)))
+        $(patsubst %,%/$(strip $1)/module-info.java,
$(IMPORT_MODULES_SRC))))

  # Find module-info.java files in the specific source dir
  # Param 1 - Src dir to find module-info.java files in

/Magnus
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8189376 Unsorted $(wildcard) causes instable module-deps.gmk

Erik Joelsson
Looks good.

/Erik


On 2017-10-25 10:09, Magnus Ihse Bursie wrote:

> We have implicitly assumed that subsequent calls to $(wildcard) will
> return the same (sorted) order of files during a single build
> operation. This is not guaranteed since GNU make 3.82, but will be the
> case in practice on most systems due the the filesystem being used.
>
> It turned out that if the order was different, FindAllModuleInfos
> returned different strings which in turn caused module-deps.gmk to be
> re-generated at any time during the build, causing race conditions.
>
> The bug analysis and patch is contributed by Martin Buchholz.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8189376
> Patch inline:
> diff --git a/make/common/Modules.gmk b/make/common/Modules.gmk
> --- a/make/common/Modules.gmk
> +++ b/make/common/Modules.gmk
> @@ -249,10 +249,10 @@
>  # configuration.
>  # Param 1 - Module to find for, set to * for finding all
>  FindAllModuleInfos = \
> -    $(wildcard \
> +    $(sort $(wildcard \
>          $(foreach sub, $(SRC_SUBDIRS), \
>            $(patsubst %,%/$(strip $1)/$(sub)/module-info.java,
> $(TOP_SRC_DIRS))) \
> -        $(patsubst %,%/$(strip $1)/module-info.java,
> $(IMPORT_MODULES_SRC)))
> +        $(patsubst %,%/$(strip $1)/module-info.java,
> $(IMPORT_MODULES_SRC))))
>
>  # Find module-info.java files in the specific source dir
>  # Param 1 - Src dir to find module-info.java files in
>
> /Magnus

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8189376 Unsorted $(wildcard) causes instable module-deps.gmk

Tim Bell
Looks good to me as well.

/Tim

On 10/25/17 01:38, Erik Joelsson wrote:

> Looks good.
>
> /Erik
>
>
> On 2017-10-25 10:09, Magnus Ihse Bursie wrote:
>> We have implicitly assumed that subsequent calls to $(wildcard) will
>> return the same (sorted) order of files during a single build
>> operation. This is not guaranteed since GNU make 3.82, but will be the
>> case in practice on most systems due the the filesystem being used.
>>
>> It turned out that if the order was different, FindAllModuleInfos
>> returned different strings which in turn caused module-deps.gmk to be
>> re-generated at any time during the build, causing race conditions.
>>
>> The bug analysis and patch is contributed by Martin Buchholz.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8189376
>> Patch inline:
>> diff --git a/make/common/Modules.gmk b/make/common/Modules.gmk
>> --- a/make/common/Modules.gmk
>> +++ b/make/common/Modules.gmk
>> @@ -249,10 +249,10 @@
>>  # configuration.
>>  # Param 1 - Module to find for, set to * for finding all
>>  FindAllModuleInfos = \
>> -    $(wildcard \
>> +    $(sort $(wildcard \
>>          $(foreach sub, $(SRC_SUBDIRS), \
>>            $(patsubst %,%/$(strip $1)/$(sub)/module-info.java,
>> $(TOP_SRC_DIRS))) \
>> -        $(patsubst %,%/$(strip $1)/module-info.java,
>> $(IMPORT_MODULES_SRC)))
>> +        $(patsubst %,%/$(strip $1)/module-info.java,
>> $(IMPORT_MODULES_SRC))))
>>
>>  # Find module-info.java files in the specific source dir
>>  # Param 1 - Src dir to find module-info.java files in


Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8189376 Unsorted $(wildcard) causes instable module-deps.gmk

Martin Buchholz-3
In reply to this post by Magnus Ihse Bursie
So we're already successfully using this patch, it solves our real world
problem, it's a simple effective solution, but it points out architectural
problems that I would want to address if I was maintaining the build
system.  I might waste too much time re-reading "Recursive Make Considered
Harmful"

On Wed, Oct 25, 2017 at 1:09 AM, Magnus Ihse Bursie <
[hidden email]> wrote:

> We have implicitly assumed that subsequent calls to $(wildcard) will
> return the same (sorted) order of files during a single build operation.
> This is not guaranteed since GNU make 3.82, but will be the case in
> practice on most systems due the the filesystem being used.
>
> It turned out that if the order was different, FindAllModuleInfos returned
> different strings which in turn caused module-deps.gmk to be re-generated
> at any time during the build, causing race conditions.
>
> The bug analysis and patch is contributed by Martin Buchholz.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8189376
> Patch inline:
> diff --git a/make/common/Modules.gmk b/make/common/Modules.gmk
> --- a/make/common/Modules.gmk
> +++ b/make/common/Modules.gmk
> @@ -249,10 +249,10 @@
>  # configuration.
>  # Param 1 - Module to find for, set to * for finding all
>  FindAllModuleInfos = \
> -    $(wildcard \
> +    $(sort $(wildcard \
>          $(foreach sub, $(SRC_SUBDIRS), \
>            $(patsubst %,%/$(strip $1)/$(sub)/module-info.java,
> $(TOP_SRC_DIRS))) \
> -        $(patsubst %,%/$(strip $1)/module-info.java,
> $(IMPORT_MODULES_SRC)))
> +        $(patsubst %,%/$(strip $1)/module-info.java,
> $(IMPORT_MODULES_SRC))))
>
>  # Find module-info.java files in the specific source dir
>  # Param 1 - Src dir to find module-info.java files in
>
> /Magnus
>