Skip to content

Commit

Permalink
gthread: consider intial connectino as idle
Browse files Browse the repository at this point in the history
this change consider the connection is iddle when started and remove it from the eventloop if no change appears in the interval. 

This prevents the connection queued for too much time.
  • Loading branch information
benoitc committed May 7, 2023
1 parent a423fca commit e7781d2
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion gunicorn/workers/gthread.py
Expand Up @@ -122,10 +122,13 @@ def accept(self, server, listener):
sock, client = listener.accept()
# initialize the connection object
conn = TConn(self.cfg, sock, client, server)
# set timeout to ensure it will not be in the loop too long
conn.set_timeout()

self.nr_conns += 1

# wait until socket is readable
with self._lock:
self._keep.append(conn)
self.poller.register(conn.sock, selectors.EVENT_READ,
partial(self.on_client_socket_readable, conn))
except EnvironmentError as e:
Expand Down

2 comments on commit e7781d2

@JamesMilneFilmlight
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Benoit,

I've run into a problem with this. This change results in each connection being added to the _keep queue twice.

The first time on_client_socket_readable() is called, it only removes the connection from _keep if the connection is initialised, but the connection is not initialised at this point, and does not get initialised until the following call to self.enqueue_req(conn).

When the request completes, finish_request() will add the connection to _keep, so it's now in the queue twice.

When murder_keepalived() is called, it hits a ValueError in self.poller.unregister(conn.sock) when it tries to remove the second copy of the connection, as the connection is already closed.

Suggest solution is to remove the connection from _keep if not is not initialised in on_client_socket_readable(), and clear the initialized flag in finish_request() before returning the connection to _keep.

@benoitc
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry i didn't get the notification. looking

Please sign in to comment.