[pve-devel] [RFC PATCH common] Tools: make file-locking aware of external exception sources

[Dietmar Maurer]

I think this can work, but $checkptr is a reference, so you should not
use it to store the value directly (see blow).

> +    my $checkptr = $lockhash->{$filename};
> +    my $local_fh; # This must stay local
> +    if (!$checkptr || !$$checkptr) {
> +	# We cannot create a weak reference in a single atomic step, so we first
> +	# create a false-value, then create a reference to it, then weaken it,
> +	# and after successfully locking the file we change the boolean value.
> +	#
> +	# The reason for this is that if an outer SIGALRM throws an exception
> +	# between creating the reference and weakening it, a subsequent call to
> +	# lock_file_full() will see a leftover full reference to a valid
> +	# variable. This variable must be 0 in order for said call to attempt to
> +	# lock the file anew.
> +	#
> +	# An externally triggered exception elsewhere in the code will cause the
> +	# weak reference to become 'undef', and since the file handle is only
> +	# stored in the local scope in $local_fh, the file will be closed by
> +	# perl's cleanup routines as well.
> +	#
> +	# This still assumes that an IO::File handle can properly deal with such
> +	# exceptions thrown during its own destruction, but that's up to perls
> +	# guts now.
> +	$checkptr = 0;
> +	$lockhash->{$filename} = \$checkptr;

I would use the following code here:
      
 my $check = 0;
 $lockhash->{$filename} = \$check;

 
> +	weaken $lockhash->{$filename};
> +	$local_fh = eval { run_with_timeout($timeout, $get_locked_file) };
> +	if ($@) {
> +	    $@ = "can't lock file '$filename' - $@";
> +	    return undef;
>  	}
> +	$checkptr = 1;

 $check = 1;

>      }
> -
> -    if ($err) {
> -        $@ = $err;
> -        return undef;
> -    }
> -
> -    $@ = undef;
> -
> +    $res = eval { &$code(@param); };
> +    return undef if $@;
>      return $res;
>  }
>  
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel