sig_block() does not nest properly

[jdemeyer]

This doesn't work as expected:

sig_block()
sig_block()
sig_unblock()
sig_unblock()

This causes warnings in Sage when cysignals is compiled in debug mode. @embray

[embray]

Would it make sense if sig_block/unblock just incremented/decremented cysigs.block_sigint rather than set it to 1/0? Then the only warning should be if there are more unblocks than blocks (it would be hard to warn about the opposite if it can be arbitrarily deeply nested).

[jdemeyer]

Right, that general idea should work. It's just that we have to think about potential race conditions here (incrementing a variable is not an atomic operation, while setting it to a fixed number is atomic).

[embray]

True--though that raises the question as to what is cysignals' story with respect to thread-safety in the first place? sig_on/off are definitely not safe as is either.

[jdemeyer]

cysignals is not thread-safe at all (that's a different topic: #21).

I'm talking about race conditions given signals (which should be handled correctly by a package which is meant to deal with signals).

[embray]

Ah I see what you're saying--just in general if a signal interrupted the increment/decrement.

[embray]

Well, GCC provides these: https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html

[jdemeyer]

Thinking about it... just incrementing/decrementing looks safe, but the reasoning why it's safe is non-trivial and needs to be documented.

[vbraun]

C++11 has atomic operations that are portable, and not just some gcc implementation details:

static std::atomic_int x = 1
x += 1   // no race condition

C11 has similar operations in , just uglier syntax.

Setting variables may or may not be atomic depending on the CPU. Eg on armeabi-v7a (basically all smartphones) you need a special ASM instruction to atomically set a 64-bit int which the compiler does not necessarily emit for a C/C++ assignment.

[jdemeyer]

https://github.com/sagemath/cysignals/commit/4801f95262cfd8cbee4b06cd6a0b67abd088f244

[embray]

I was thinking that as well--as long as no actual signal handlers touch the sig_block value it doesn't matter if the increment is interrupted by a signal. There's still a race condition, technically, but one that would exist either way.