search

HPCE / hpce-2017-cw6

2 stars 17 forks source link

possible thread-safe issue #44

Open jeg114 opened 6 years ago

jeg114 commented 6 years ago

I have been blindly using util_Assert declared as a macro in testu01/mylib/util.cpp:107 (probably should have checked its use before), until I have run into some issues with it.

Turns out it was just an inverted condition, but during exploring it I did notice that it uses exit(int status) which according to this reference should not be used when running multiple threads. The same reference suggests quick_exit as an alternative or _Exit can be I believe, since it does not perform the cleanup tasks which I believe can be the issues of exit.

Im still unsure how exit would make a multithreaded program crash and have not observed it. I could not find a decent explanation online either. Maybe someone can expand on the topic.

m8pple commented 6 years ago

That wouldn't have even occurred to me. So I suppose the failure mode would be something like: 1 - Static object is initialised somewhere. e.g. some kind of object that writes a file or does some other side-effecting activity when it is destroyed. So something like RAII, but via global vars. 2 - Two tasks manage to call exit at exactly the same time - unlikely, but possible. 3 - Both threads attempt to destruct the shared object, which means the side-effect actually happens twice. For example, we might see the same message written to a file twice or something. Or we could try to free the same thing twice, which would also cause a problem.

Unlike a lot of other races, it has negligible probability of actually happening, as the occurrence of exit has to be so rare within a program (each thread can only ever do it once). Within the context of util_Assert it also happens when the program is getting torn down anyway, so strange behaviour is possible anyway. It's notable that the normal version of assert explicitly exits by abort, which is thread safe.

My inclination would be to replace with quick_exit, although abort also has some appeal.

While it does seem to be a genuine concurrency bug (as util_Assert might fail in two different functions in normal use), it is an incredibly obscure one and difficult to actually exercise - though that does make it interesting.

On that basis, I'll give 0.5% bounty, as it has a slightly similar feel to

12, but this one is a lot more hidden.