Skip to content

Commit

Permalink
shm_sub BUGFIX clear left over event and orig_cid
Browse files Browse the repository at this point in the history
orig_cid should always be cleared when the originator stops waiting for
the subscribers. If not, subsequent events cannot be generated on the
subscription SHM, until the previous originator process is restarted.

Normally, the wait logic takes care of this. However, in case a
subscription evpipe could not be notified we bail on waiting and return
with error. However, in this case orig_cid and event are left behind
never to be cleared.
  • Loading branch information
irfanHaslanded authored and michalvasko committed Jul 3, 2024
1 parent 1773ced commit ed1a659
Showing 1 changed file with 39 additions and 0 deletions.
39 changes: 39 additions & 0 deletions src/shm_sub.c
Original file line number Diff line number Diff line change
Expand Up @@ -1353,6 +1353,11 @@ sr_shmsub_change_notify_update(struct sr_mod_info_s *mod_info, const char *orig_
goto cleanup;

cleanup_wrunlock:
/* clear the event unless a subscriber reported an error, in which case further clean up will happen */
if (sub_shm->event != SR_SUB_EV_ERROR) {
ATOMIC_STORE_RELAXED(sub_shm->event, SR_SUB_EV_NONE);
sub_shm->orig_cid = 0;
}
/* SUB WRITE UNLOCK */
sr_rwunlock(&sub_shm->lock, 0, SR_LOCK_WRITE, cid, __func__);

Expand Down Expand Up @@ -1637,6 +1642,12 @@ sr_shmsub_change_notify_change(struct sr_mod_info_s *mod_info, const char *orig_
cleanup:
for (i = 0; i < notify_count; ++i) {
if (notify_subs[i].lock) {
/* clear the event unless a subscriber reported an error and an abort will be sent */
if (notify_subs[i].sub_shm->event != SR_SUB_EV_ERROR) {
ATOMIC_STORE_RELAXED(notify_subs[i].sub_shm->event, SR_SUB_EV_NONE);
notify_subs[i].sub_shm->orig_cid = 0;
}

/* SUB UNLOCK */
sr_rwunlock(&notify_subs[i].sub_shm->lock, 0, notify_subs[i].lock, cid, __func__);
notify_subs[i].lock = SR_LOCK_NONE;
Expand Down Expand Up @@ -1798,6 +1809,10 @@ sr_shmsub_change_notify_change_done(struct sr_mod_info_s *mod_info, const char *
cleanup:
for (i = 0; i < notify_count; ++i) {
if (notify_subs[i].lock) {
/* always clear the event */
ATOMIC_STORE_RELAXED(notify_subs[i].sub_shm->event, SR_SUB_EV_NONE);
notify_subs[i].sub_shm->orig_cid = 0;

/* SUB UNLOCK */
sr_rwunlock(&notify_subs[i].sub_shm->lock, 0, notify_subs[i].lock, cid, __func__);
notify_subs[i].lock = SR_LOCK_NONE;
Expand Down Expand Up @@ -2021,6 +2036,10 @@ sr_shmsub_change_notify_change_abort(struct sr_mod_info_s *mod_info, const char
cleanup:
for (i = 0; i < notify_count; ++i) {
if (notify_subs[i].lock) {
/* always clear the event */
ATOMIC_STORE_RELAXED(notify_subs[i].sub_shm->event, SR_SUB_EV_NONE);
notify_subs[i].sub_shm->orig_cid = 0;

/* SUB UNLOCK */
sr_rwunlock(&notify_subs[i].sub_shm->lock, 0, notify_subs[i].lock, cid, __func__);
notify_subs[i].lock = SR_LOCK_NONE;
Expand Down Expand Up @@ -2188,6 +2207,10 @@ sr_shmsub_oper_get_notify(struct sr_mod_info_mod_s *mod, const char *xpath, cons
cleanup:
for (i = 0; i < notify_count; ++i) {
if (notify_subs[i].lock) {
/* clear any event left behind due to an error (eg. could not notify evpipe) */
ATOMIC_STORE_RELAXED(notify_subs[i].sub_shm->event, SR_SUB_EV_NONE);
notify_subs[i].sub_shm->orig_cid = 0;

/* SUB UNLOCK */
sr_rwunlock(&notify_subs[i].sub_shm->lock, 0, notify_subs[i].lock, cid, __func__);
notify_subs[i].lock = SR_LOCK_NONE;
Expand Down Expand Up @@ -2701,6 +2724,12 @@ sr_shmsub_rpc_notify(sr_conn_ctx_t *conn, sr_rwlock_t *sub_lock, off_t *subs, ui
} while (subscriber_count);

cleanup_wrunlock:
/* clear the event unless a subscriber reported an error and an abort will be sent */
if (sub_shm->event != SR_SUB_EV_ERROR) {
sub_shm->event = SR_SUB_EV_NONE;
sub_shm->orig_cid = 0;
}

/* SUB WRITE UNLOCK */
sr_rwunlock(&sub_shm->lock, 0, SR_LOCK_WRITE, conn->cid, __func__);

Expand Down Expand Up @@ -2830,6 +2859,10 @@ sr_shmsub_rpc_notify_abort(sr_conn_ctx_t *conn, sr_rwlock_t *sub_lock, off_t *su
return err_info;

cleanup_wrunlock:
/* always clear the event */
ATOMIC_STORE_RELAXED(sub_shm->event, SR_SUB_EV_NONE);
sub_shm->orig_cid = 0;

/* SUB WRITE UNLOCK */
sr_rwunlock(&sub_shm->lock, 0, SR_LOCK_WRITE, conn->cid, __func__);

Expand Down Expand Up @@ -2967,6 +3000,12 @@ sr_shmsub_notif_notify(sr_conn_ctx_t *conn, const struct lyd_node *notif, struct
goto cleanup;

cleanup_ext_sub_unlock:
/* always clear the event if an error occurred */
if (err_info) {
ATOMIC_STORE_RELAXED(sub_shm->event, SR_SUB_EV_NONE);
sub_shm->orig_cid = 0;
}

/* SUB WRITE UNLOCK */
sr_rwunlock(&sub_shm->lock, 0, SR_LOCK_WRITE, conn->cid, __func__);

Expand Down

0 comments on commit ed1a659

Please sign in to comment.