Skip to content

Non thread-safe use of setrounding in construction of BigFloat from Rational #52859

Description

The method for constructing a BigFloat from a Rational uses setrounding in a way that is not thread-safe. I could consistently reproduce the issue with the following code:

r = 1 // 3
x = (BigFloat(r, RoundDown), BigFloat(r, RoundUp))
Threads.@threads for i in 1:10000
    y = BigFloat(r, ifelse(isodd(i), RoundDown, RoundUp))
    @assert x[mod1(i, 2)] == y
end

If running Julia with only one thread the assert is always ok, but if using 2 (or more) threads it fails (most of the time).

My Julia version is

julia> versioninfo()
Julia Version 1.10.0
Commit 3120989f39b (2023-12-25 18:01 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 8 × 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, tigerlake)
  Threads: 2 on 8 virtual cores

The issue is this function in MPFR.jl

function BigFloat(x::Rational, r::MPFRRoundingMode=ROUNDING_MODE[]; precision::Integer=DEFAULT_PRECISION[])
    setprecision(BigFloat, precision) do
        setrounding_raw(BigFloat, r) do
            BigFloat(numerator(x))::BigFloat / BigFloat(denominator(x))::BigFloat
        end
    end
end

The use of setrounding_raw is not thread safe. In fact we can notice that setprecision is also not used in a thread-safe way. I think this can be fairly easily fixed by calling the mpfr_div function directly, and specifying the rounding mode. I would be happy to prepare a PR for this, but wanted to check if people agree with my analysis first. I can also note that his method is the only use of setprecision and setrounding in MPFR.jl, so this should be the only problematic place.

However, there is also the question of what rounding means in this situation. Currently the rounding is applied on the conversion of the numerator and denominator as well as on the division. This is not necessarily the same as rounding the mathematically exact rational to the nearest representable floating point. I don't think there is a good solution to this in general though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    bignumsBigInt and BigFloatbugIndicates an unexpected problem or unintended behaviormultithreadingBase.Threads and related functionalityrationalsThe Rational type and values thereof

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions