Is this thread safe?

[Jason House]

I think the following should work.  Note the use of a temporary when
rebuilding the table and the two volatile variables.


real logFactorial(uint n) {
    static volatile real* factorial_table = ([0.0L]).ptr;
    static volatile size_t length = 1;
    //Length is guaranteed to never decrease.
    //At least at an abstract level, the synchronized part is guaranteed
    to //only write to elements >= length.
    if(length > n) {  // Length can only get larger
        return factorial_table[n]; // factorial_table can only get longer
    }
    synchronized {
      if (n >= length){
        real* new_factorial_table
        if(capacity(factorial_table) < (n + 1) * real.sizeof) {
            new factorial_table = cast(real*) realloc(factorial_table, 
            (n + 1) * real.sizeof);
        }
        else {
            new_factorial_table = factorial_table;
        }
        for(uint i = length; i<=n; i++) {
            new_factorial_table[i] = factorial_table[i-1] + log2(i);
        }
        factorial_table = new_factorial_table; // safe, length too short
        length = n; // safe now that factorial_table is set
      }
    }
    return factorial_table[n];
}



dsimcha wrote:

> From reading over these links, it makes sense to me why my previous
> implementation would not work. However, the articles, which were written
> with C++, not D, in
> mind, recommend using the volatile keyword to solve this.  In D, volatile
> is deprecated, even though, according to microbenchmarks I just tried, it
> has much
> less overhead than synchronized.  I also understand from reading previous
> forum
> posts that it applies to statements, not variables.  If my understanding
> of volatile is correct, my function could be made correct as follows, by
> simply inserting a volatile at the statement that reads factorial_table*.
> 
> real logFactorial(uint n) {
>     static real* factorial_table = ([0.0L]).ptr;
>     static size_t length = 1;
>     //Length is guaranteed to never decrease.
>     //At least at an abstract level, the synchronized part is guaranteed
>     to //only write to elements >= length.
>     if(length > n) {  //Length can only get larger, cached length is fine.
>         volatile real result = factorial_table[n]; //Doesn't use cached
> factorial_table*
>         return result;
>     }
>     synchronized {
>         if(capacity(factorial_table) < (n + 1) * real.sizeof) {
>             //Is it safe to update the factorial_table pointer like this?
>             factorial_table = cast(real*) realloc(factorial_table, (n + 1)
>             *
> real.sizeof);
>         }
>         for(uint i = length; i<=n; i++) {
>             factorial_table[i] = factorial_table[i-1] + log2(i);
>         }
>         //Set new length after all numbers are calculated.
>         length = n;
>         return factorial_table[n];
>     }
> }
> 
> Other than the fact that volatile is deprecated, would this be a correct
> fix?  If
> so, why is volatile deprecated?  If not, is it not correct?