<p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -2</span></p><p><a href="https://gerrit.osmocom.org/13971">View Change</a></p><p>1 comment:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/13971/1/stp/stp_main.c">File stp/stp_main.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13971/1/stp/stp_main.c@221">Patch Set #1, Line 221:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">/* Gracefully shut down SIGTRAN links */<br>        osmo_signal_dispatch(SS_L_GLOBAL, S_L_GLOBAL_SHUTDOWN, NULL);<br> sleep(1); /* FIXME: do we really need to sleep() here? */<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">actually, that won't work.  All our code is running asynchronously from the select main loop.  So if the signal handlers are starting some activitiy that doesn't immediately timeout (e.g. sending an ASP-DOWN to the peer and waiting for the ACK), your sleep here won't help.</p><p style="white-space: pre-wrap; word-wrap: break-word;">If we want to do graceful shutdown, we should</p><ul><li>receive the signal in the handler</li><li>change some state (such as quit++)</li><li>continue in the main loop</li><li>send the signal from the main loop</li><li>give the code some time (e.g. start a timer)</li><li>shut down on timer expiration</li></ul><p style="white-space: pre-wrap; word-wrap: break-word;">If this fails for some reason, a second signal could be used to do quit++ and then cause a hard exit.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/13971">change 13971</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/13971"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmo-sccp </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I5d8618857c5119d4acca5d65cf1276ab02889c84 </div>
<div style="display:none"> Gerrit-Change-Number: 13971 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Sun, 12 May 2019 10:15:47 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>