mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 12:32:43 +00:00 
			
		
		
		
	RequestServer: Avoid multiple connections to incompatible servers
This really only implements a heuristic, assuming that HTTP/1.0 servers cannot handle having multiple active connections; this assumption has lots of false positives, but ultimately HTTP/1.0 is an out-of-date HTTP version and people using it should just switch to a newer standard anyway. Specifically, python's "SimpleHTTPRequestHandler" utilises a single-threaded HTTP/1.0 server, which means no keepalive and more importantly, hangs and races with more than a single connection present. This commit makes it so we serialise all requests to servers that are known to serve only a single request per connection (aka HTTP/1.0 with our setup, as we unconditionally request keepalive)
This commit is contained in:
		
							parent
							
								
									e770cf06b0
								
							
						
					
					
						commit
						cd4ebc45a0
					
				
					 2 changed files with 23 additions and 2 deletions
				
			
		|  | @ -13,6 +13,7 @@ namespace RequestServer::ConnectionCache { | ||||||
| 
 | 
 | ||||||
| HashMap<ConnectionKey, NonnullOwnPtr<Vector<NonnullOwnPtr<Connection<Core::TCPSocket, Core::Socket>>>>> g_tcp_connection_cache {}; | HashMap<ConnectionKey, NonnullOwnPtr<Vector<NonnullOwnPtr<Connection<Core::TCPSocket, Core::Socket>>>>> g_tcp_connection_cache {}; | ||||||
| HashMap<ConnectionKey, NonnullOwnPtr<Vector<NonnullOwnPtr<Connection<TLS::TLSv12>>>>> g_tls_connection_cache {}; | HashMap<ConnectionKey, NonnullOwnPtr<Vector<NonnullOwnPtr<Connection<TLS::TLSv12>>>>> g_tls_connection_cache {}; | ||||||
|  | HashMap<ByteString, InferredServerProperties> g_inferred_server_properties; | ||||||
| 
 | 
 | ||||||
| void request_did_finish(URL const& url, Core::Socket const* socket) | void request_did_finish(URL const& url, Core::Socket const* socket) | ||||||
| { | { | ||||||
|  | @ -37,6 +38,10 @@ void request_did_finish(URL const& url, Core::Socket const* socket) | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         auto& connection = *connection_it; |         auto& connection = *connection_it; | ||||||
|  |         auto& properties = g_inferred_server_properties.ensure(partial_key.hostname); | ||||||
|  |         if (!connection->socket->is_open()) | ||||||
|  |             properties.requests_served_per_connection = min(properties.requests_served_per_connection, connection->max_queue_length + 1); | ||||||
|  | 
 | ||||||
|         if (connection->request_queue.is_empty()) { |         if (connection->request_queue.is_empty()) { | ||||||
|             // Immediately mark the connection as finished, as new jobs will never be run if they are queued
 |             // Immediately mark the connection as finished, as new jobs will never be run if they are queued
 | ||||||
|             // before the deferred_invoke() below runs otherwise.
 |             // before the deferred_invoke() below runs otherwise.
 | ||||||
|  |  | ||||||
|  | @ -98,6 +98,7 @@ struct Connection { | ||||||
|     Core::ElapsedTimer timer {}; |     Core::ElapsedTimer timer {}; | ||||||
|     JobData job_data {}; |     JobData job_data {}; | ||||||
|     Proxy proxy {}; |     Proxy proxy {}; | ||||||
|  |     size_t max_queue_length { 0 }; | ||||||
| }; | }; | ||||||
| 
 | 
 | ||||||
| struct ConnectionKey { | struct ConnectionKey { | ||||||
|  | @ -120,8 +121,13 @@ struct AK::Traits<RequestServer::ConnectionCache::ConnectionKey> : public AK::De | ||||||
| 
 | 
 | ||||||
| namespace RequestServer::ConnectionCache { | namespace RequestServer::ConnectionCache { | ||||||
| 
 | 
 | ||||||
|  | struct InferredServerProperties { | ||||||
|  |     size_t requests_served_per_connection { NumericLimits<size_t>::max() }; | ||||||
|  | }; | ||||||
|  | 
 | ||||||
| extern HashMap<ConnectionKey, NonnullOwnPtr<Vector<NonnullOwnPtr<Connection<Core::TCPSocket, Core::Socket>>>>> g_tcp_connection_cache; | extern HashMap<ConnectionKey, NonnullOwnPtr<Vector<NonnullOwnPtr<Connection<Core::TCPSocket, Core::Socket>>>>> g_tcp_connection_cache; | ||||||
| extern HashMap<ConnectionKey, NonnullOwnPtr<Vector<NonnullOwnPtr<Connection<TLS::TLSv12>>>>> g_tls_connection_cache; | extern HashMap<ConnectionKey, NonnullOwnPtr<Vector<NonnullOwnPtr<Connection<TLS::TLSv12>>>>> g_tls_connection_cache; | ||||||
|  | extern HashMap<ByteString, InferredServerProperties> g_inferred_server_properties; | ||||||
| 
 | 
 | ||||||
| void request_did_finish(URL const&, Core::Socket const*); | void request_did_finish(URL const&, Core::Socket const*); | ||||||
| void dump_jobs(); | void dump_jobs(); | ||||||
|  | @ -173,12 +179,21 @@ ErrorOr<void> recreate_socket_if_needed(T& connection, URL const& url) | ||||||
| decltype(auto) get_or_create_connection(auto& cache, URL const& url, auto job, Core::ProxyData proxy_data = {}) | decltype(auto) get_or_create_connection(auto& cache, URL const& url, auto job, Core::ProxyData proxy_data = {}) | ||||||
| { | { | ||||||
|     using CacheEntryType = RemoveCVReference<decltype(*cache.begin()->value)>; |     using CacheEntryType = RemoveCVReference<decltype(*cache.begin()->value)>; | ||||||
|     auto& sockets_for_url = *cache.ensure({ url.serialized_host().release_value_but_fixme_should_propagate_errors().to_byte_string(), url.port_or_default(), proxy_data }, [] { return make<CacheEntryType>(); }); | 
 | ||||||
|  |     auto hostname = url.serialized_host().release_value_but_fixme_should_propagate_errors().to_byte_string(); | ||||||
|  |     auto& properties = g_inferred_server_properties.ensure(hostname); | ||||||
|  | 
 | ||||||
|  |     auto& sockets_for_url = *cache.ensure({ move(hostname), url.port_or_default(), proxy_data }, [] { return make<CacheEntryType>(); }); | ||||||
| 
 | 
 | ||||||
|     Proxy proxy { proxy_data }; |     Proxy proxy { proxy_data }; | ||||||
| 
 | 
 | ||||||
|     using ReturnType = decltype(sockets_for_url[0].ptr()); |     using ReturnType = decltype(sockets_for_url[0].ptr()); | ||||||
|     auto it = sockets_for_url.find_if([](auto& connection) { return connection->request_queue.is_empty(); }); |     // Find the connection with an empty queue; if none exist, we'll find the least backed-up connection later.
 | ||||||
|  |     // Note that servers that are known to serve a single request per connection (e.g. HTTP/1.0) usually have
 | ||||||
|  |     // issues with concurrent connections, so we'll only allow one connection per URL in that case to avoid issues.
 | ||||||
|  |     // This is a bit too aggressive, but there's no way to know if the server can handle concurrent connections
 | ||||||
|  |     // without trying it out first, and that's not worth the effort as HTTP/1.0 is a legacy protocol anyway.
 | ||||||
|  |     auto it = sockets_for_url.find_if([&](auto& connection) { return properties.requests_served_per_connection < 2 || connection->request_queue.is_empty(); }); | ||||||
|     auto did_add_new_connection = false; |     auto did_add_new_connection = false; | ||||||
|     auto failed_to_find_a_socket = it.is_end(); |     auto failed_to_find_a_socket = it.is_end(); | ||||||
|     if (failed_to_find_a_socket && sockets_for_url.size() < ConnectionCache::MaxConcurrentConnectionsPerURL) { |     if (failed_to_find_a_socket && sockets_for_url.size() < ConnectionCache::MaxConcurrentConnectionsPerURL) { | ||||||
|  | @ -253,6 +268,7 @@ decltype(auto) get_or_create_connection(auto& cache, URL const& url, auto job, C | ||||||
|     } else { |     } else { | ||||||
|         dbgln_if(REQUESTSERVER_DEBUG, "Enqueue request for URL {} in {} - {}", url, &connection, connection.socket); |         dbgln_if(REQUESTSERVER_DEBUG, "Enqueue request for URL {} in {} - {}", url, &connection, connection.socket); | ||||||
|         connection.request_queue.append(decltype(connection.job_data)::create(job)); |         connection.request_queue.append(decltype(connection.job_data)::create(job)); | ||||||
|  |         connection.max_queue_length = max(connection.max_queue_length, connection.request_queue.size()); | ||||||
|     } |     } | ||||||
|     return &connection; |     return &connection; | ||||||
| } | } | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Ali Mohammad Pur
						Ali Mohammad Pur