Grisu (floating point printing) not thread-safe

[oschulz]

It looks like string interpolation is currently not threadsafe. Running the following script "string_interpolation_threadsafe_test.jl"

using Base.Threads


function test_with_mutex()
	a = rand(1000)
	m = Mutex()
	@threads for i=1:1000
		y = lock(m) do
			"$(a[i])"
		end
		@assert a[i] == parse(Float64, y)
	end
end


function test_without_mutex()
	a = rand(1000)
	@threads for i=1:1000
		y = "$(a[i])"
		@assert a[i] == parse(Float64, y)
	end
end


function test_without_mutex_repeated()
	for i in 1:1000
		a = rand(1000)
		@threads for i=1:1000
			y = "$(a[i])"
		end
	end
end


@info "test_with_mutex()"
test_with_mutex()

@info "test_without_mutex()"
test_without_mutex()

@info "test_without_mutex_repeated()"
test_without_mutex_repeated()

via

JULIA_NUM_THREADS=4 julia string_interpolation_threadsafe_test.jl

with Julia 0.7.0-DEV.3551 (x86_64, official binary, on Ubuntu 16.04) results in

[ Info: test_with_mutex()
[ Info: test_without_mutex()

Error thrown in threaded loop on thread 1: AssertionError(msg="a[i] == parse(Float64, y)")
Error thrown in threaded loop on thread 0: AssertionError(msg="a[i] == parse(Float64, y)")
Error thrown in threaded loop on thread 3: AssertionError(msg="a[i] == parse(Float64, y)")
Error thrown in threaded loop on thread 2: AssertionError(msg="a[i] == parse(Float64, y)")

[ Info: test_without_mutex_repeated()

Error thrown in threaded loop on thread 0: InexactError(func=:trunc, T=UInt8, val=-1)
Error thrown in threaded loop on thread 3: DivideError()
Error thrown in threaded loop on thread 0: DivideError()
Error thrown in threaded loop on thread 1: DivideError()
Error thrown in threaded loop on thread 0: InexactError(func=:trunc, T=UInt8, val=-1)
Error thrown in threaded loop on thread 1: InexactError(func=:trunc, T=UInt8, val=-1)

test_without_mutex_repeated() sometimes ends in a segfault.

The same problem occurs with Julia v0.6.

As string interpolation may be used deep inside a third-party function/dependency, and is therefore hard to control/fence, this is a serious problem when writing multi-threaded code.

[oschulz]

Discovered by @RCS-77.

[yuyichao]

String interpolation is fine, the global buffer for floating point formatting in grisu is not.

[oschulz]

Ah, thanks for the clarification, @yuyichao!

[oschulz]

Could we simply do something like

const DIGITS = [Vector{UInt8}(uninitialized, 309+17) for _ in 1:Base.Threads.nthreads()]

[...]

function grisu(v::AbstractFloat,mode,requested_digits,buffer=DIGITS[Base.Threads.threadid()],bignums=BIGNUMS)
    if signbit(v)

[...]
...

in "grisu.jl"?

[Keno]

We have more allocation elision for buffers now, so we should probably just move it inside the function. The one wrinkle is that I believe this buffer is shared across quite a number of large functions, so our local escape analysis is probably not powerful enough. I wonder whether we can just start doing inter-procedural escape analysis in inference.

[vtjnash]

I wonder whether we can just start doing inter-procedural escape analysis in inference.

Hopefully soon. I'm not sure if that'll work for our Arrays right now as well. Might need to change this to a Ref{NTuple{309 + 17, UInt8}}() to make it a fixed size, but that brings about its own complications. Currently there's also the additional complication though that BigInt calls resize! on this buffer too, which requires that it be heap allocated.

[JeffBezanson]

Yes, I think the uses in grisu are going to be very hard to optimize automatically.

[oschulz]

Since it's a fixed size buffer - could we simply stack allocate 309+17 bytes and get the pointer - or does it have to be a real Julia array?

[oschulz]

If stack allocation is not possible and allocation elision won't work in this scenario yet, could we adopt the "one array per thread" approach for now, to get thread safety? Maybe also as a backport to v0.6? Non-threadsafe floating point formatting is kinda painful to work around ...

[KlausC]

Please keep in mind, that also printf.jl is accessing DIGITS directly.

[oschulz]

Thanks, @stevengj !