Hi Konstantin,
Konstantin Osipov, 05.05.2009 22:21:
> * Ingo Strüwing <Ingo.Struewing@stripped> [09/05/05 23:35]:
...
> your patch is bound to solve the present day trouble only unless
> you write an instruction to developers that they can follow
> when writing new code: an instruction how to write "killable" and
> race-free code.
Where shall this instruction got to? Where is the old instruction?
I tried to include the instruction at every wait condition. I think that
a developer, who wants to write "killable" code, will look at other
places where similar things are done. So I hope, he will find one of the
repeated comments. But I've already received the comment that these many
repetitions don't seem useful. Now I'm gathering opinions, which place
might be optimal for a single copy.
>
> When you move the checks for thd->killed into critical section,
> you change the conceptual model of thd->enter_cond().
> According to the old conceptual model it was unnecessary, since
> thd->awake cancels the wait on the condition (at least such was
> the pitch).
It does still cancel the wait on the condition. I don't understand what
you mean with me to "change the conceptual model of thd->enter_cond()".
As far as I understand the "conceptual model of thd->enter_cond()", it
is not affected in its main feature: Making it possible to kick a thread
out of a wait condition in an exceptional way. thd->killed is just one
way to transport the exception itself. The way, how to check it, may be
seen as a peripheral recommendation. If you think, I'm wrong, please
explain in detail, what is the conceptual model is and why the
(additional) check for thd->killed changes the model.
>
> thd->killed has never been too reliable. It wasn't designed to
> be. In real life a user can send the signal twice without too much
> trouble. But the way it worked was simple and easy to understand.
My aim is to make thd->killed more reliable. If only to support
regression testing.
I'm sorry to hear that you don't find it simple and easy to understand
that the check for thd->killed should not (only) be done before
enter_cond(), but (also) after it.
>
> So, before we proceed, can we zoom out from the code for a minute
> and discuss:
>
> - what was the old model for thd->killed. What was wrong with it?
I didn't know that we had a "model" for thd->killed. Can you please
point me at its written instruction/documentation?
The way thd->killed was examined, was unnecessary unreliable. Checking
it before enter_cond() only allows for a race condition.
> - what is the new model. How is it better?
Checking thd->killed (also) after enter_cond() makes it reliable
regarding wait conditions. IMHO this is a simple and inexpensive
addition, which improves reliability of thd->killed.
I agree that this may not be desirable for real-life applications as
they (the user) can repeat the kill request.
But it improves the testability of KILL. Without this change we probably
need to live with a failing kill.test, or not (regression-)testing KILL
at all.
Regards
Ingo
--
Ingo Strüwing, Database Group
Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schröder, Wolfgang Engels, Wolf Frenkel
Vorsitzender des Aufsichtsrates: Martin Häring HRB München 161028