lchan_fsm: safer 'concluded' flag
The flag lchan->activate.concluded prevents entering the lchan_on_fully_established()/lchan_on_activation_failure() more than once. So far it was checked by callers, instead place in the functions themselves. There is a potential functional change here, since during lchan_fail(), the concluded flag was set only after entering lchan_on_activation_failure(). Now it is already set at the start and prevents multiple re-entry beyond doubt. This patch is not a result of an actual observed faulty behavior, just from reading the code and seeing the slight loophole. Change-Id: I0c906438b05442d66255203eb816453b7193a6d8
This commit is contained in:
parent
e23e1aeec2
commit
193c1e3ab0
|
@ -505,7 +505,10 @@ struct gsm_lchan {
|
||||||
struct {
|
struct {
|
||||||
enum lchan_activate_mode activ_for;
|
enum lchan_activate_mode activ_for;
|
||||||
bool activ_ack; /*< true as soon as RSL Chan Activ Ack is received */
|
bool activ_ack; /*< true as soon as RSL Chan Activ Ack is received */
|
||||||
bool concluded; /*< true as soon as LCHAN_ST_ESTABLISHED is reached */
|
/*! This flag ensures that when an lchan activation has succeeded, and we have already
|
||||||
|
* sent ACKs like Immediate Assignment or BSSMAP Assignment Complete, and if other errors
|
||||||
|
* occur later, e.g. during release, that we don't send a NACK out of context. */
|
||||||
|
bool concluded;
|
||||||
bool requires_voice_stream;
|
bool requires_voice_stream;
|
||||||
bool wait_before_switching_rtp; /*< true = requires LCHAN_EV_READY_TO_SWITCH_RTP */
|
bool wait_before_switching_rtp; /*< true = requires LCHAN_EV_READY_TO_SWITCH_RTP */
|
||||||
uint16_t msc_assigned_cic;
|
uint16_t msc_assigned_cic;
|
||||||
|
|
|
@ -88,6 +88,10 @@ static void _lchan_on_activation_failure(struct gsm_lchan *lchan, enum lchan_act
|
||||||
struct gsm_subscriber_connection *for_conn,
|
struct gsm_subscriber_connection *for_conn,
|
||||||
const char *file, int line)
|
const char *file, int line)
|
||||||
{
|
{
|
||||||
|
if (lchan->activate.concluded)
|
||||||
|
return;
|
||||||
|
lchan->activate.concluded = true;
|
||||||
|
|
||||||
switch (activ_for) {
|
switch (activ_for) {
|
||||||
|
|
||||||
case FOR_MS_CHANNEL_REQUEST:
|
case FOR_MS_CHANNEL_REQUEST:
|
||||||
|
@ -135,6 +139,10 @@ static void _lchan_on_activation_failure(struct gsm_lchan *lchan, enum lchan_act
|
||||||
|
|
||||||
static void lchan_on_fully_established(struct gsm_lchan *lchan)
|
static void lchan_on_fully_established(struct gsm_lchan *lchan)
|
||||||
{
|
{
|
||||||
|
if (lchan->activate.concluded)
|
||||||
|
return;
|
||||||
|
lchan->activate.concluded = true;
|
||||||
|
|
||||||
switch (lchan->activate.activ_for) {
|
switch (lchan->activate.activ_for) {
|
||||||
case FOR_MS_CHANNEL_REQUEST:
|
case FOR_MS_CHANNEL_REQUEST:
|
||||||
/* No signalling to do here, MS is free to use the channel, and should go on to connect
|
/* No signalling to do here, MS is free to use the channel, and should go on to connect
|
||||||
|
@ -222,9 +230,7 @@ struct state_timeout lchan_fsm_timeouts[32] = {
|
||||||
lchan_set_last_error(_lchan, "lchan %s in state %s: " fmt, \
|
lchan_set_last_error(_lchan, "lchan %s in state %s: " fmt, \
|
||||||
_lchan->activate.concluded ? "failure" : "allocation failed", \
|
_lchan->activate.concluded ? "failure" : "allocation failed", \
|
||||||
osmo_fsm_state_name(fsm, state_was), ## args); \
|
osmo_fsm_state_name(fsm, state_was), ## args); \
|
||||||
if (!_lchan->activate.concluded) \
|
lchan_on_activation_failure(_lchan, _lchan->activate.activ_for, _lchan->conn); \
|
||||||
lchan_on_activation_failure(_lchan, _lchan->activate.activ_for, _lchan->conn); \
|
|
||||||
_lchan->activate.concluded = true; \
|
|
||||||
if (fi->state != state_chg) \
|
if (fi->state != state_chg) \
|
||||||
lchan_fsm_state_chg(state_chg); \
|
lchan_fsm_state_chg(state_chg); \
|
||||||
else \
|
else \
|
||||||
|
@ -751,10 +757,6 @@ static void lchan_fsm_established_onenter(struct osmo_fsm_inst *fi, uint32_t pre
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* This flag ensures that when an lchan activation has succeeded, and we have already sent ACKs
|
|
||||||
* like Immediate Assignment or BSSMAP Assignment Complete, and if then, way later, some other
|
|
||||||
* error occurs, e.g. during release, that we don't send a NACK out of context. */
|
|
||||||
lchan->activate.concluded = true;
|
|
||||||
lchan_on_fully_established(lchan);
|
lchan_on_fully_established(lchan);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue