View Issue Details

IDProjectCategoryView StatusLast Update
0007772libmicrohttpdperformancepublic2024-01-21 13:33
Reporternewtec_mh Assigned ToChristian Grothoff  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionreopened 
Product Version0.9.75 
Fixed in Version0.9.76 
Summary0007772: Upgraded connection in threaded context is starved of memory
DescriptionI'm experiencing an issue with upgraded connections running libmicrohttpd with a thread per connection. What seemingly happens is that after receiving the headers, the connections read buffers are shrunk, and the write buffer is afterwards increased to the full size of the memory pool.

This is never "undone" meaning that *if* a connection is upgraded, the memory pool is "empty" and the upgraded connection then starts using the emergency buffer (e_buf) which by default has a size of 8 bytes, 4 is allocated to write, 4 to read.

This means suddenly all communication between the application and the library and again between the library and the upgraded connection socket is segmented into 4 byte reads/writes.

Now as such, this doesn't necessarily post a "big" problem, besides it for sure isn't intended functionality, however if the connections are https based, each 4 byte "chunk" is now encrypted, causing a massive overhead (in my case each 4 byte chunk is then encrypted a 26 byte chunk).

The seemingly "obvious" choice I found for "easily" fixing this, at least in my case, was to reenable the disabled function "connection_shrink_write_buffer" and call if after the headers have been sent, just before upgrading the connection. I've attached a patch for reference that fixes it in one case at least. I have not determined other possible code paths that might trigger the same problem.
Steps To Reproduce1. Create server with MHD_USE_THREAD_PER_CONNECTION and MHD_ALLOW_UPGRADE.

2. Connect WebSocket to server.

3. Observe the buffer size used in the connection handler thread (through GDB for instance). Read/write buffers are 4 bytes each, as the e_buf "emergency buffer" is used, which defaults to 8 bytes in size.
TagsNo tags attached.
Attached Files
avoid-upgraded-connection-memory-starvation.patch (1,035 bytes)   
Index: libmicrohttpd-0.9.76/src/microhttpd/connection.c
===================================================================
--- libmicrohttpd-0.9.76.orig/src/microhttpd/connection.c
+++ libmicrohttpd-0.9.76/src/microhttpd/connection.c
@@ -1612,8 +1612,6 @@ connection_maximize_write_buffer (struct
   return c->write_buffer_size - c->write_buffer_append_offset;
 }
 
-
-#if 0 /* disable unused function */
 /**
  * Shrink connection write buffer to the size of unsent data.
  *
@@ -1654,10 +1652,6 @@ connection_shrink_write_buffer (struct M
     c->write_buffer = new_buf;
 }
 
-
-#endif /* unused function */
-
-
 /**
  * Switch connection from recv mode to send mode.
  *
@@ -4700,6 +4694,8 @@ MHD_connection_handle_idle (struct MHD_C
 #ifdef UPGRADE_SUPPORT
       if (NULL != connection->response->upgrade_handler)
       {
+        connection_shrink_write_buffer(connection);
+
         connection->state = MHD_CONNECTION_UPGRADE;
         /* This connection is "upgraded".  Pass socket to application. */
         if (MHD_NO ==

Activities

Christian Grothoff

2023-09-03 00:50

manager   ~0020471

Sorry for the huge delay with this one. Fixed exactly as suggested in 438c3ee2..fabafbaa.

Karlson2k

2023-09-03 10:10

developer   ~0020476

Actually it was reported in mailinglist and was fixed previously by da42f22c64357179f3869594ca4001c7525a2a1f
https://lists.gnu.org/archive/html/libmicrohttpd/2023-04/msg00001.html

Issue History

Date Modified Username Field Change
2023-03-24 15:24 newtec_mh New Issue
2023-03-24 15:24 newtec_mh File Added: avoid-upgraded-connection-memory-starvation.patch
2023-09-03 00:50 Christian Grothoff Assigned To => Christian Grothoff
2023-09-03 00:50 Christian Grothoff Status new => resolved
2023-09-03 00:50 Christian Grothoff Resolution open => fixed
2023-09-03 00:50 Christian Grothoff Fixed in Version => Git master
2023-09-03 00:50 Christian Grothoff Note Added: 0020471
2023-09-03 10:10 Karlson2k Status resolved => feedback
2023-09-03 10:10 Karlson2k Resolution fixed => reopened
2023-09-03 10:10 Karlson2k Note Added: 0020476
2023-12-02 12:10 Christian Grothoff Status feedback => resolved
2023-12-02 12:10 Christian Grothoff Fixed in Version Git master => 0.9.76
2024-01-21 13:33 Christian Grothoff Status resolved => closed