Is uiThread required to avoid data races?

[niklasf]

Apologies if #1990 was closed intentionally. If so, I'd appreciate brief explanantion.

---

uiThread is used here (a):

Stockfish/src/uci.cpp

Line 196 in 6514500

pos.set(StartFEN, false, &states->back(), uiThread.get());

But not here (b):

Stockfish/src/uci.cpp

Line 70 in 6514500

pos.set(fen, Options["UCI_Chess960"], &states->back(), Threads.main());

This means that there's either a data race in (b), or an upportunity to simplify away uiThread in (a). (Or a third option, that I am missing). Can anyone figure out which it is?

[vondele]

the rationale for uiThread is in the commit message 750dfa0 . If I understand correctly to allow certain commands while search is going on. That's a bit of a grey area, since UCI suggests that this is not conforming. So, the one at line 196 would be a feature, while the one at line 70 is OK for conforming uci commands. This is tricky code, I would recommend to compile with the sanitize=thread option, and see what happens when issuing UCI commands during search.

[xoto10]

@niklasf
The comment vondele refers to says the problem is a data race if eval is requested while searching. It seems to me we could just deny this eval request, e.g. see my branch uithread2

[vondele]

yes, I did a quick check, and even with this uiThread in place, things are not race free issuing an eval during an ongoing search. It might have been a partial fix, but not the full fix. In that case, it is not clear if there is any benefit from the code.

[vondele]

this issue can be closed now.

[ddugovic]

@niklasf Merged to our fork at https://travis-ci.org/ddugovic/Stockfish/builds/600211626

[niklasf]

Thanks all.