Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

I may be wrong, but I thought that in a multithreaded environment, doing i++ is not atomic and could result in garbled data. Instead you should use __sync_add_and_fetch. However, I have no idea if it should be used inside abort().


>I may be wrong, but I thought that in a multithreaded environment, doing i++ is not atomic and could result in garbled data. Instead you should use __sync_add_and_fetch. However, I have no idea if it should be used inside abort().

This is only true if the variable in question's memory is accessed by multiple threads at the same time, and there isn't any locking or synchronization method used to protect the memory.

In this case, even though it is a globally scoped variable, it's locked by the globally scoped mutex declared in the file. All increments are done in the locked sections, so there isn't any possibility of accessing the variable without having a lock.

It should be noted that there is a very minor race condition when abort() is called in two different threads sequentially, and every attempt up to line 89 doesn't work. The first call will get the lock, then go through to line 89, where it released the lock. The second thread will then get the lock, and go through the first section. When it hits the section line 89(if (been_there_done_that == 0)), that will resolve to false, because been_there_done_that is 1. It will then go on, leaving the first thread deadlocked at the LOCK attempt on line 91. This shouldn't result in any missed functionality, but I actually wonder why they're releasing the lock in the first place. Raise() isn't thread safe anyway, because the signal is applied to all threads in the process. Plus, you're trying to suicide the program. It's a bad idea to even have the possibility of multiple threads trying to kill themselves at the same time.


The lock must be released because the same thread might reenter abort() in the signal handler, and without a release in the parent abort(), the program would hang.


You are right. Re-reading the code, I see that it re-acqures the lock.


Since the lock is not guaranteed in the code, the variable is globally defined and the code only ever increases it. This means a step in the chain of killing could get skipped, but that doesn't matter, as there's always a more violent option (or just the infinite loop).


On an 8-bit system with 16-bit-wide ints, i++ is almost certainly not atomic (with respect to threads, interrupts, etc.).

A few months ago I fixed this exact bug in a developer's code, on a 8-bit embedded processor.


i++ is atomic in most cases, unless i is an excessively wide integer, in abort I think they lock anyway so it doesn't matter (at least in uclibc)


Actually, while the write itself is generally atomic, most systems don't guarantee that the entire increment operation is (for performance reasons).

And since abort still needs to work even without locks and on every platform...




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: