echo not thread safe on windows, causing [appveyor] flaky test: Failure: reOutputsDiffer in tforstmt.nim

[timotheecour]

this just happened in https://ci.appveyor.com/project/dom96/nim/build/3984

FAIL: tforstmt.nim C
Test "tests\parallel\tforstmt.nim" in category "parallel"
Failure: reOutputsDiffer
Expected:
3
4
5
6
7
Gotten:
3
4
5
76

tforstmt.nim:

discard """
  output: '''3
4
5
6
7'''
  sortoutput: true
"""

import threadpool, os

proc p(x: int) =
  os.sleep(100 - x*10)
  echo x

proc testFor(a, b: int; foo: var openArray[int]) =
  parallel:
    for i in max(a, 0) .. min(b, foo.high):
      spawn p(foo[i])

var arr = [0, 1, 2, 3, 4, 5, 6, 7]

testFor(3, 10, arr)

the bug is that 2 echo statements have overlapped, causing 76 to print at once instead of each ech obeing interleaved

not sure how frequently that happens, but the fact that it did happen in appveyor shows that echo isn't thread safe

[awr1]

Is it ever claimed anywhere that echo is thread-safe? This parallels the same behavior as one would expect from printf or std::cout in C/C++.

[timotheecour]

Is it ever claimed anywhere that echo is thread-safe?

https://nim-lang.org/docs/system.html#echo,varargs%5Btyped,%5D

Unlike other IO operations this is guaranteed to be thread-safe as echo is very often used for debugging convenience.

This parallels the same behavior as one would expect from printf or std::cout in C/C++.

for that, use stdout.write("foo") etc

[skilchen]

@timotheecour cited Nim's documentation:

https://nim-lang.org/docs/system.html#echo,varargs%5Btyped,%5D

Unlike other IO operations this is guaranteed to be thread-safe as echo is very often used for debugging convenience.

This was never true on Windows. Commit 527e20f makes it evident. flockfile/funlockfile do not exist in the c-runtime-libraries used on Windows.

[Araq]

This was never true on Windows. Commit 527e20f makes it evident. flockfile/funlockfile do not exist in the c-runtime-libraries used on Windows.

Yeah, it's pretty bad. The Windows implementation needs to adhere to the documentation. Probably we need to use an echoLock on our own for this.