[OE-core] [PATCH 1/3] pseudo: Handle too many files deadlock

[Gary Thomas]

On 2017-06-14 15:42, Richard Purdie wrote:
> If we have large amounts of parallelism, pseudo can end up with too
> many open connections and will no longer accept further connections,
> hanging. This patch works around that by closing some clients, allowing
> turnover of connections and unblocking the system. The downside is a small
> but theoretical window of data loss. This is likely better than locking
> up entirely though. Discussions with Peter are onging about how we could
> better fix this.
> 
> Signed-off-by: Richard Purdie <richard.purdie at linuxfoundation.org>
> ---
>   .../pseudo/files/toomanyfiles.patch                | 62 ++++++++++++++++++++++
>   meta/recipes-devtools/pseudo/pseudo_1.8.2.bb       |  1 +
>   2 files changed, 63 insertions(+)
>   create mode 100644 meta/recipes-devtools/pseudo/files/toomanyfiles.patch
> 
> diff --git a/meta/recipes-devtools/pseudo/files/toomanyfiles.patch b/meta/recipes-devtools/pseudo/files/toomanyfiles.patch
> new file mode 100644
> index 0000000..14a8975
> --- /dev/null
> +++ b/meta/recipes-devtools/pseudo/files/toomanyfiles.patch
> @@ -0,0 +1,62 @@
> +Currently if we max out the maximum number of files, pseudo can deadlock, unable to
> +accept new connections yet unable to move forward and unblock the other processes
> +waiting either.
> +
> +Rather than hang, when this happens, close out inactive connections, allowing us
> +to accept the new ones. The disconnected clients will simply reconnect. There is
> +a small risk of data loss here sadly but its better than hanging.
> +
> +RP
> +2017/4/25
> +
> +Upstream-Status: Pending [Peter is aware of the issue]
> +
> +Index: pseudo-1.8.2/pseudo_server.c
> +===================================================================
> +--- pseudo-1.8.2.orig/pseudo_server.c
> ++++ pseudo-1.8.2/pseudo_server.c
> +@@ -581,6 +581,7 @@ pseudo_server_loop(void) {
> + 	int rc;
> + 	int fd;
> + 	int loop_timeout = pseudo_server_timeout;
> ++	int hitmaxfiles;
> +
> + 	clients = malloc(16 * sizeof(*clients));
> +
> +@@ -597,6 +598,7 @@ pseudo_server_loop(void) {
> + 	active_clients = 1;
> + 	max_clients = 16;
> + 	highest_client = 0;
> ++	hitmaxfiles = 0;
> +
> + 	pseudo_debug(PDBGF_SERVER, "server loop started.\n");
> + 	if (listen_fd < 0) {
> +@@ -663,10 +665,18 @@ pseudo_server_loop(void) {
> + 						message_time.tv_usec -= 1000000;
> + 						++message_time.tv_sec;
> + 					}
> ++				} else if (hitmaxfiles) {
> ++					/* In theory there is a potential race here where if we close a client,
> ++					   it may have sent us a fastop message which we don't act upon.
> ++					   If we don't close a filehandle we'll loop indefinitely thought.
> ++					   Only close one per loop iteration in the interests of caution */
> ++					close_client(i);
> ++					histmaxfiles = 0;

Typo?  s/histmaxfiles/hitmaxfiles/

> + 				}
> + 				if (die_forcefully)
> + 					break;
> + 			}
> ++			hitmaxfiles = 0;
> + 			if (!die_forcefully &&
> + 			    (FD_ISSET(clients[0].fd, &events) ||
> + 			     FD_ISSET(clients[0].fd, &reads))) {
> +@@ -688,6 +698,9 @@ pseudo_server_loop(void) {
> +                                          */
> +                                         pseudo_server_timeout = DEFAULT_PSEUDO_SERVER_TIMEOUT;
> +                                         die_peacefully = 0;
> ++				} else if (errno == EMFILE) {
> ++					hitmaxfiles = 1;
> ++					pseudo_debug(PDBGF_SERVER, "Hit max open files, dropping a client.\n");
> + 				}
> + 			}
> + 			pseudo_debug(PDBGF_SERVER, "server loop complete [%d clients left]\n", active_clients);
> diff --git a/meta/recipes-devtools/pseudo/pseudo_1.8.2.bb b/meta/recipes-devtools/pseudo/pseudo_1.8.2.bb
> index b427b9a..9bcd031 100644
> --- a/meta/recipes-devtools/pseudo/pseudo_1.8.2.bb
> +++ b/meta/recipes-devtools/pseudo/pseudo_1.8.2.bb
> @@ -7,6 +7,7 @@ SRC_URI = "http://downloads.yoctoproject.org/releases/pseudo/${BPN}-${PV}.tar.bz
>              file://moreretries.patch \
>              file://efe0be279901006f939cd357ccee47b651c786da.patch \
>              file://b6b68db896f9963558334aff7fca61adde4ec10f.patch \
> +           file://toomanyfiles.patch \
>              "
>   
>   SRC_URI[md5sum] = "7d41e72188fbea1f696c399c1a435675"
> 


-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------