Protect against spurious wakeups

[elfring]

I have seen that no loop is used around a call of the function "pthread_cond_wait".
Would you like to reuse anything from my message on the topic "spurious wakeup"?

[cloudwu]

The worker thread calling pthread_cond_wait do some simple work: it checks the message queue continuously , if no new message , it sleeps awhile , otherwise it dispatch a message from the queue.

Pthread_cond_wait is not important and not necessary. It's in the loop of message dispatching, and message queue is thread safe, we can do the job any times even if message queue is empty. So it don't care about why it be waked up.

[elfring]

I would prefer that the shown programming mistake will be corrected. The predicate which should be checked together with the functiion "pthread_cond_wait" should be specified.

[cloudwu]

I don't care why and when pthread_cond_wait could be wake up, even remove pthread_cond_wait is also right.
Why must check it in a loop ?

[elfring]

I guess that you want also to wait (put a thread to sleep) only if a specific condition was not present.

[cloudwu]

The signal to wake up slept thread will be send by timer continuously without any condition.
And the worker threads seldom sleep in my system, because the message queues are always not empty.

[elfring]

Would you like to wait for the predicate that "a queue" is empty or full?

[cloudwu]

No. Send signal will kill the performance and increase the complexity .

The queue seldom empty. I check the queue directly in the worker thread loop like a spin lock.

[elfring]

You use this programming interface in an incomplete way. Would you like to recheck any recommendations from other information sources?

[cloudwu]

If you read the source code, you can find skynet use a 2 level message queue. The only way to know whether the queue is empty is try to pop a message.

And I don't care whether the signal send out or the thread capture the signal.

If you don't like it, you can simply remove all the pthread_wait_cond and pthread_cond_signal, the program will works fine, too.

[elfring]

Why do you use such functions when you expect that this program should also work without them?

[cloudwu]

Sometimes there are not so many works to do, so some workers can sleep a while , it's optional.

There are many worker thread in skynet, and the work of threads are the same, so I don't care which one sleeps and which one wake up.

[elfring]

How do you think about to become a bit more aware of the proper handling for the involved predicates?

[cloudwu]

the queue seldom empty, so the worker thread simply do again in any situation even the error raised from pthread api is ok.

Do more is harmless to this system and the code will be simple.

[elfring]

I prefer software correctness over "simplicity" for this use case.

[cloudwu]

I think it's correct now because any error from pthread_cond_wait is
harmless.

2013/9/1 Markus Elfring notifications@github.com

I prefer software correctness over "simplicity" for this use case.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-23626740
.

http://blog.codingnow.com

[elfring]

No. - I see still a need for further improvements to make the applied algorithms really correct.

[cloudwu]

If you can find how and when it is wrong, you can make a test case to show it.

In this place, pthread_cond_wait can be call once or more times or remove it (don't call it), the algorithms are always correct. So I don't care it success or failed.

[elfring]

I find your view questionable.

• Such a failed function call might mean that an other thread tampered with the referenced data structures in unexpected ways.
• I get the impression that you should reconsider the predicate handling (e. g. loops for condition checks) in this software.

[cloudwu]

I guess you don't read other parts of code, so you don't know why I can ignore the failed function call.

pthread_cond_wait here only let the cpu/thread sleep a while , nothing else. and pthread_cond_signal only suggest (not must) it can wake up.

As I said before, sleeping is optional. Whether the sleeping really happened is not important.

[elfring]

Would you like to delete unneeded source code?

[cloudwu]

It's not unneeded . Just like a cpu, if it detects few work to do , it works at lower frequency automatic. Whether it works at low or high frequency, both can do work correctly .

[elfring]

If you do not want to get rid of the "optional sleeping", I would prefer that your use of condition variables will become complete and correct.

[cloudwu]

Do you mean use semaphore (sem__) instead of pthread_cond__ ? I think no difference between them to my requirement , but pthread api is more portable.

[elfring]

I do not suggest to change to an alternative function interface here. It seems that your desire for a thread which should sleep occasionally does not fit to a correct implementation so far. I hope that the needed predicates will become clearer for your software.

[cloudwu]

The thread can sleep or not sleep as it like, the signal is only the suggestion. so it doesn't care the predicate.
I think the code is simple and clear enough now, and it's correct. If it is wrong, you can make a test case to show it .

[elfring]

I have got a different opinion. I can not show "an error demonstration" by a test case for this special situation. The real problem seems to be the detail that you do not care for the needed predicates as much as I try to recommend here. I hope that other software developers can eventually convince you to add specific loops around each call of the function "pthread_cond_wait".

[cloudwu]

I am sure that whatever the "pthread_cond_wait" returns (even raise an error) , the worker thread can do things right. So I don't need capture the error.

[elfring]

Please distinguish the issues "completion of error handling" and "correct use of condition variables".

Do you expect the semantics "wake-only-once" here?

[cloudwu]

No. As I said before, removing "pthread_cond_wait" is no harmful either . So wake up multi-times is ok.

[elfring]

Why are you not going to remove the "pthread_cond_..." stuff then?

[cloudwu]

I explained many times, so i don't want to say more. Read the code if you want to know the reason.

[elfring]

I miss still your real explanation so far.

[elfring]

For which conditions would you like to wait by the discussed variable?

[cloudwu]

The "pthread_cond_wait" here only sleeps a while and maybe wake up by "pthread_cond_signal"
I don't care about "spurious wakeup" because the main loop of worker thread check queue continuously without any condition.

Worker threads can do their jobs in any condition. they may sleep a while when they find not many jobs need to do. The number of jobs is not the condition either. In the worst case, every threads sleep a little time and signals are not send out immediately, and the messages delay a while. But the system can still works fine (every message will be dispatch later).

Maybe use eventfd or semaphore is better than pthread_cond because there is no condition need to check here. Do you have any new suggestion ?

[elfring]

Condition variables are not the appropriate means to wait without regarding predicates.

• Drop the "pthread_cond_..." stuff from the affected source file to make it cleaner.
• If you insist to use them, I recommend to add corresponding loops with dedicated checks.

[cloudwu]

I don't need checks, and nothing need check.

I can't find another pthread api can do this.

You never told me if I don't check what's the bad thing will happen. I know my system clearly and I know what's the "spurious wakeup". I recommend you to read the source code and then give a pull request or fork the project to modify it as you like.

[elfring]

I do not know the intended software design good enough to offer a Git pull request. But I interpret the affected source code in the way that the predicate handling should be improved. I find it bad to overlook relevant conditions.