<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On 2011-08-23, at 03:48 AM, Henrik Nord wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>On 08/03/2011 05:07 PM, Frédéric Trottier-Hébert wrote:<br><blockquote type="cite">git fetch git@github.com:ferd/otp.git fix_supervisor_temporary_restart<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">This patch fixes the issue discussed at <a href="http://erlang.org/pipermail/erlang-questions/2011-August/060419.html">http://erlang.org/pipermail/erlang-questions/2011-August/060419.html</a>, namely that in one_for_all or rest_for_one strategies, the supervisor is still trying to restart temporary children. Because we don't track their MFAs entirely, this causes repetitive crashes.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">This patch fixes the behaviour by inserting a clause in terminate_children/2-3 (private function) that will omit temporary children when building a list of killed processes, to avoid having the supervisor trying to restart them again.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Only supervisors in need of restarting children used the list, so the change should be of no impact for the functions that called terminate_children/2-3 only to kill all children. This also avoids useless iterations if the filtering step were to be inserted at any other point in the code.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">A test suite has been added to supervisor_SUITE, containing checks for both one_for_all and rest_for_one. I've only ran the tests for the suite itself, but all of them pass without a problem.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">The commit/diff itself: <a href="https://github.com/ferd/otp/commit/25aab8efdef5c6898a679705158627cded3f847a">https://github.com/ferd/otp/commit/25aab8efdef5c6898a679705158627cded3f847a</a><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">--<br></blockquote><blockquote type="cite">Fred Hébert<br></blockquote><blockquote type="cite"><a href="http://www.erlang-solutions.com">http://www.erlang-solutions.com</a><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">_______________________________________________<br></blockquote><blockquote type="cite">erlang-patches mailing list<br></blockquote><blockquote type="cite"><a href="mailto:erlang-patches@erlang.org">erlang-patches@erlang.org</a><br></blockquote><blockquote type="cite"><a href="http://erlang.org/mailman/listinfo/erlang-patches">http://erlang.org/mailman/listinfo/erlang-patches</a><br></blockquote><br>Hello<br><br>Your patch has been reviewed, and we want you to fix the following for it to be graduated.<br><br>1) I think the test needs to be extended a bit. I can't see that it checks what happens to the temporary child - i.e. that it is terminated but not restarted. As far as I can understand it only checks that the supervisor survives...<br><br>2) There is a minor typo in the description of the test - it says one_for_rest instead of rest_for_one.<br><br>3) The behaviour must be documented - i.e. it must be explicitly stated that temporary children under one_for_all or rest_for_one supervisors will be terminated but not restarted.<br><br>/siri<br><br><br>Thank you.<br><br>-- <br>/Henrik Nord Erlang/OTP<br><br></div></blockquote></div><br>1) Good catch. The broken behaviour would crash the supervisor -- and I tested that this was solved, but not that the child isn't retarted. This has been fixed.<br><br>2) also fixed<br><br>3) I changed it. I wasn't too sure where to put it, but there was only one place where temporary children were discussed so I added it there.<br><br>I've amended the commit: <a href="https://github.com/ferd/otp/commit/4f9b938112a80f1c1e210b1a6af40a42f833cfa2">https://github.com/ferd/otp/commit/4f9b938112a80f1c1e210b1a6af40a42f833cfa2</a><br><br>The fetch command was: git fetch git@github.com:ferd/otp.git fix_supervisor_temporary_restart<br><br>Thanks for your time,<br><br>--<br>Fred Hébert<br><a href="http://www.erlang-solutions.com">http://www.erlang-solutions.com</a></body></html>