[Libwebsockets] Patch - don't touch revents after deleting FD

Andy Green andy at warmcat.com
Fri Jun 12 02:16:21 CEST 2015



On 06/11/2015 03:02 AM, Bruce Perens wrote:
> Andy seems to be away and not moderating tickets, trac is still broken,
> and I just want to get these off my table.

Currently I am in a deathmatch with my paying work.  I tend to have an 
embarrassment of time for this or no time at all.

> This fixes a problem in which pollfd->revents was cleared after a socket
> was deleted, thus clearing the wrong FD. This might be part of the
> reason that libwebsockets has been dropping events, but not all of it.
> I'm still chasing one.
>
>      Thanks
>
>      Bruce
>
> diff -Naur libwebsockets-old/lib/service.c libwebsockets/lib/service.c
> --- libwebsockets-old/lib/service.c2015-06-07 15:25:03.768514858 -0700
> +++ libwebsockets/lib/service.c2015-06-10 11:59:02.259679461 -0700
> @@ -360,8 +360,6 @@
> int m;
> int listen_socket_fds_index = 0;
> time_t now;
> -int timed_out = 0;
> -int our_fd = 0;
> char draining_flow = 0;
> int more;
> struct lws_tokens eff_buf;
> @@ -384,32 +382,19 @@
> lws_plat_service_periodic(context);
> /* global timeout check once per second */
> -
> -if (pollfd)
> -our_fd = pollfd->fd;

Well the patch is a bit mangled.  I guess it's my problem, although 
github is also usable for this.

> for (n = 0; n < context->fds_count; n++) {
> m = context->fds[n].fd;
> wsi = wsi_from_fd(context,m);
> if (!wsi)
> continue;
> -if (libwebsocket_service_timeout_check(context, wsi, now))
> -/* he did time out... */
> -if (m == our_fd) {
> -/* it was the guy we came to service! */
> -timed_out = 1;
> -/* mark as handled */
> -if (pollfd)
> -pollfd->revents = 0;
> -}
> +if (libwebsocket_service_timeout_check(context, wsi, now)) {
> +                          /* The socket was closed and *pollfd is
> invalid */
> +                          return 0;

This is no longer a "global timeout" then.  If you have many clients, 
and more than one are timing out, the timeout for the ones with higher 
fd could be infinitely delayed by guys timing out with lower fds.

I think I see your general desire there but isn't it continue; ?  And 
then we need the logic to deal with the specific fd we wanted timed out 
if it got called like that.

-Andy

> +                        }
> }
> }
> -/* the socket we came to service timed out, nothing to do */
> -if (timed_out)
> -return 0;
> -
> /* just here for timeout management? */
> if (pollfd == NULL)
> return 0;
> @@ -608,9 +593,11 @@
>   close_and_handled:
> lwsl_debug("Close and handled\n");
> +        /* Warning: *pollfd is invalid after this call, don't touch it! */
> libwebsocket_close_and_free_session(context, wsi,
> LWS_CLOSE_STATUS_NOSTATUS);
> n = 1;
> +        return n;
>   handled:
> pollfd->revents = 0;
>
>
>
> _______________________________________________
> Libwebsockets mailing list
> Libwebsockets at ml.libwebsockets.org
> http://ml.libwebsockets.org/mailman/listinfo/libwebsockets
>



More information about the Libwebsockets mailing list