[Libwebsockets] Multi-tail ring-buffer's oldest_tail issue

Andy Green andy at warmcat.com
Tue Nov 27 14:49:49 CET 2018



On 27/11/2018 21:38, Ariel D'Alessandro wrote:
> Hi,
> 
> I have a question related to the multi-tail ring-buffer implementation.
> Specifically on what happens with the ring-buffer's oldest_tail when the
> object with *that* tail is removed from the list of objects.
> 
> As you can see in some of the minimal-examples, e.g.:
> 
>      minimal-examples/ws-server/minimal-ws-broker/protocol_lws_minimal.c
> 
> when the client connection is closed, the object is removed from the
> list of living objects, but the ring-buffer is not updated at all.
> 
>      case LWS_CALLBACK_CLOSED:
>          /* remove our closing pss from the list of live pss */
>          lws_ll_fwd_remove(struct per_session_data__minimal, pss_list,
>                            pss, vhd->pss_list);
>          break;
> 
> On the other hand, elements on the ring-buffer are consumed by a call to:
> 
>      lws_ring_consume_and_update_oldest_tail(
>          vhd->ring,
>          struct per_session_data__minimal,
>          &pss->tail,
>          1,
>          vhd->pss_list,
>          tail,
>          pss_list
>      );
> 
> which updates the oldest_tail based on the living objects' tails. But it
> will only update the oldest_tail if the actual consumer *is* the
> oldest_tail:
> 
>      #define lws_ring_consume_and_update_oldest_tail(\
>          [ ... ]
>          ) { \
>              int ___n, ___m; \
>          \
>          ___n = lws_ring_get_oldest_tail(___ring) == *(___ptail); \
>          lws_ring_consume(___ring, ___ptail, NULL, ___count); \
>          if (___n) { \
>              [ ... update oldest_tail ... ]
>          } \
>      }
> 
> So, the issue I see is the following sequence:
> * Object A is the *only* living object with a tail matching the
> ring-buffer's oldest_tail.
> * Object A is removed from the list of living objects, without updating
> the ring-buffer's oldest tail (as in the minimal-example above).
> * Further calls to lws_ring_consume_and_update_oldest_tail on the list
> of living objects won't update the ring-buffer's oldest_tail as nobody
> has a tail matching it.
> 
> Does this make any sense? Shold we be "consuming" all its waiting
> elements before removing the object from list of living objects?
> 
>      case LWS_CALLBACK_CLOSED:
>          lws_ring_consume_and_update_oldest_tail(
>              vhd->ring,
>              struct per_session_data__minimal,
>              &pss->tail,
>              /* Consume all waiting elements within this tail. */
>              lws_ring_get_count_waiting_elements(vhd->ring, &pss->tail),
>              vhd->pss_list,
>              tail,
>              pss_list
>          );
>          /* remove our closing pss from the list of live pss */
>          lws_ll_fwd_remove(struct per_session_data__minimal, pss_list,
>                            pss, vhd->pss_list);
>          break;

It's a bug in that particular minimal example... the most evolved, 
hammered and commented user of lws_ring is the mirror plugin, the corner 
case you noticed is handled fine there:

	case LWS_CALLBACK_CLOSED:
		/* detach our pss from the mirror instance */
		mi = pss->mi;
		if (!mi)
			break;

		lws_pthread_mutex_lock(&v->lock); /* vhost lock { */

		/* remove our closing pss from its mirror instance list */
		lws_ll_fwd_remove(struct per_session_data__lws_mirror,
				  same_mi_pss_list, pss, mi->same_mi_pss_list);
		pss->mi = NULL;

		if (mi->same_mi_pss_list) {
			/*
			 * Still other pss using the mirror instance.  The pss
			 * going away may have had the oldest tail, reconfirm
			 * using the remaining pss what is the current oldest
			 * tail.  If the oldest tail moves on, this call also
			 * will re-enable rx flow control when appropriate.
			 */
			lws_pthread_mutex_lock(&mi->lock); /* mi lock { */
			__mirror_update_worst_tail(mi);
			lws_pthread_mutex_unlock(&mi->lock); /* } mi lock */
			lws_pthread_mutex_unlock(&v->lock); /* } vhost lock */
			break;
		}
...

https://libwebsockets.org/git/libwebsockets/tree/plugins/protocol_lws_mirror.c#n285-311

It's not that lws_ring itself misses the trick, it's just very difficult 
to keep 50+ minimal examples synchronized with everything.

-Andy

> Thanks,
> 


More information about the Libwebsockets mailing list