Re: [PATCH] localedata: Avoid concurrently written locales in gen-locale.sh

Florian Weimer <[email protected]> writes:

> There is no cross-directory exclusion of concurrent $(gen-locales)
> usage.  Parallel localedef calls can clobber locale data as it is
> being loaded by tests.
>
> With --no-hard-links, the separate touch invocation is no longer
> required.
>
> ---
>  localedata/gen-locale.sh | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/localedata/gen-locale.sh b/localedata/gen-locale.sh
> index 4762c04284..b0ce27fbae 100644
> --- a/localedata/gen-locale.sh
> +++ b/localedata/gen-locale.sh
> @@ -32,15 +32,23 @@ generate_locale ()
>    out=$3
>    flags=$4
>    ret=0
> +
> +  # Use a staging area to avoid writing to locales concurrently.
> +  # While this process is running, $$ is sufficiently unique.

Yes, this should be OK.

> +  # Use --no-hard-links to prevent localedef from accessing
> +  # other staging areas.
> +  stage="${common_objpfx}localedata/gen-locale.$$.tmp"
> +
>    ${localedef_before_env} ${run_program_env} I18NPATH=../localedata \
> -	${localedef_after_env} $flags -f $charmap -i $input \
> -	${common_objpfx}localedata/$out || ret=$?
> -  if [ $ret -eq 0 ]; then
> -    # The makefile checks the timestamp of the LC_CTYPE file,
> -    # but localedef won't have touched it if it was able to
> -    # hard-link it to an existing file.
> -    touch ${common_objpfx}localedata/$out/LC_CTYPE
> +	${localedef_after_env} $flags --no-hard-links -f $charmap -i $input \
> +	$stage || ret=$?

This could be improved to directly test the command but it can be done
later, no need to do it now.

> +  if [ $ret -eq 0 ] ; then
> +      # Ignore errors in case some other process has created the same
> locale.

OK. 

> +      # (Some versions of mv do not report an error here.)
> +      mv -Tn $stage ${common_objpfx}localedata/$out 2>/dev/null || true

I'm sorry to ask this, but is mv -T sufficiently portable? It's not
POSIX but I don't recall if we require any extensions for coreutils
elsewhere in glibc.

> +      rm -rf $stage
>    else
> +    rm -rf $stage
>      echo "Charmap: \"${charmap}\" Inputfile: \"${input}\"" \
>  	 "Outputdir: \"${out}\" failed"
>      exit 1

The patch otherwise looks good, but please wait a brief period for
Adhemerval to comment before committing.

sam

-----BEGIN PGP SIGNATURE-----

iQEBBAEWCgCpFiEEJaa7iN2bdkxrVUHCc4QJ9SDfkZAFAmpOQDEbFIAAAAAABAAO
bWFudTIsMi41KzEuMTIsMiwyXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25z
Lm9wZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXQyNUE2QkI4OEREOUI3NjRDNkI1NTQx
QzI3Mzg0MDlGNTIwREY5MTkwDxxzYW1AZ2VudG9vLm9yZwAKCRBzhAn1IN+RkLPw
AP9BJb6SExOuUFB0b9Wy5IVRg7jZxCpKnUyh3C4Dgsw4hQD/djHAnXPMD78FLYjV
lkeDjJWqsYRa5SU5jk4MBG7KTwM=
=IA4a
-----END PGP SIGNATURE-----