View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0005260libmicrohttpdotherpublic2018-01-20 15:152021-09-02 17:54
Reporterghaderer Assigned ToChristian Grothoff  
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version0.9.58 
Target Version0.9.59Fixed in Version0.9.59 
Summary0005260: deadlock when chunked body transmission fails
DescriptionWhen a response's callback is called, MHD holds its mutex internally. When the callback returns a transmission error, MHD closes the connection and tries to destroy the response by calling MHD_destroy_response(). This leads to a deadlock as tries it tries to acquire the response's mutex again.

MHD must first unlock the response's mutex before closing the connection as it does in the non-chunked body case.

I join a patch which does exactly that.
Steps To ReproduceThe following example helps reproduce this defect.

<<<<
#include <assert.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

#include <microhttpd.h>

static ssize_t content_reader(
    void * data,
    uint64_t pos,
    char * buf,
    size_t max
) {
  int mode = (int)data;

  if (mode == 0) {
    buf[0] = 'x';
    return 1;
  }

  return MHD_CONTENT_READER_END_WITH_ERROR;
}

static int access_handler(
    void * data,
    struct MHD_Connection * connection,
    const char * url,
    const char * method,
    const char * version,
    const char * upload_data,
    size_t * upload_data_size,
    void ** connection_data
) {
  struct MHD_Response * response;
  int mode;
  int result;

  if (strcmp(url, "/fail") == 0)
    mode = 1;
  else
    mode = 0;

  response = MHD_create_response_from_callback(MHD_SIZE_UNKNOWN, 2048,
      content_reader, (void *)mode, NULL);
  assert(response != NULL);

  result = MHD_queue_response(connection, 200, response);
  assert(result == MHD_YES);

  *connection_data = response;

  return MHD_YES;
}

int main() {
  struct MHD_Daemon * daemon;

  daemon = MHD_start_daemon(MHD_USE_INTERNAL_POLLING_THREAD |
      MHD_USE_DEBUG, 8081,
      NULL, NULL,
      access_handler, NULL,
      MHD_OPTION_END);
  assert(daemon != NULL);

  while (1)
    sleep(1000);

  return 0;
}
>>>> example.c

Compile "example.c".

Start "example".

Run these commands:
- curl -o /dev/null http://127.0.0.1:8081 -> OK
- curl -o /dev/null http://127.0.0.1:8081 -> OK
- curl -o /dev/null http://127.0.0.1:8081/fail -> deliberate failure
- curl -o /dev/null http://127.0.0.1:8081 -> no reply as MHD's loop is in a deadlock
TagsNo tags attached.
Attached Files
fix-deadlock-when-destroying-response.patch (1,952 bytes)    
From 5e8dad7d2d44891098f2108859a408b9b80b0a58 Mon Sep 17 00:00:00 2001
From: ghaderer <ghaderer@wyplay.com>
Date: Sat, 20 Jan 2018 14:55:43 +0100
Subject: [PATCH] Fix deadlock when destroying response on chunked body
 response's callback error.

---
 src/microhttpd/connection.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/microhttpd/connection.c b/src/microhttpd/connection.c
index a85e56e..2865df2 100644
--- a/src/microhttpd/connection.c
+++ b/src/microhttpd/connection.c
@@ -1155,6 +1155,7 @@ try_ready_chunked_body (struct MHD_Connection *connection)
           size /= 2;
           if (size < 128)
             {
+              MHD_mutex_unlock_chk_ (&response->mutex);
               /* not enough memory */
               CONNECTION_CLOSE_ERROR (connection,
 				      _("Closing connection (out of memory)\n"));
@@ -1200,6 +1201,7 @@ try_ready_chunked_body (struct MHD_Connection *connection)
     {
       /* error, close socket! */
       response->total_size = connection->response_write_position;
+      MHD_mutex_unlock_chk_ (&response->mutex);
       CONNECTION_CLOSE_ERROR (connection,
 			      _("Closing connection (application error generating response)\n"));
       return MHD_NO;
@@ -1218,6 +1220,7 @@ try_ready_chunked_body (struct MHD_Connection *connection)
   if (0 == ret)
     {
       connection->state = MHD_CONNECTION_CHUNKED_BODY_UNREADY;
+      MHD_mutex_unlock_chk_ (&response->mutex);
       return MHD_NO;
     }
   if (ret > 0xFFFFFF)
@@ -3593,7 +3596,7 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
                 socket_start_no_buffering (connection);
               continue;
             }
-          MHD_mutex_unlock_chk_ (&connection->response->mutex);
+          /* mutex was already unlocked by try_ready_chunked_body */
           break;
         case MHD_CONNECTION_BODY_SENT:
           if (MHD_NO == build_header_response (connection))
-- 
2.7.4
fix-deadlock-when-destroying-response.patch (1,952 bytes)   

Activities

Christian Grothoff

2018-01-29 17:35

manager   ~0012823

Fixed as suggested in b155d866..2dd1b43e. Sorry for taking so long!

Christian Grothoff

2021-09-02 17:54

manager   ~0018182

Fix committed to master branch.

Related Changesets

libmicrohttpd: master 2dd1b43e

2018-01-29 18:35

Christian Grothoff


Details Diff		 fix 0005260 as suggested by reporter Affected Issues
0005260
mod - ChangeLog Diff File
mod - src/include/microhttpd.h Diff File
mod - src/microhttpd/connection.c Diff File

Issue History

Date Modified Username Field Change
2018-01-20 15:15 ghaderer New Issue
2018-01-20 15:15 ghaderer File Added: fix-deadlock-when-destroying-response.patch
2018-01-25 07:41 Christian Grothoff Assigned To => Christian Grothoff
2018-01-25 07:41 Christian Grothoff Status new => assigned
2018-01-29 17:35 Christian Grothoff Note Added: 0012823
2018-01-29 17:37 Christian Grothoff Status assigned => resolved
2018-01-29 17:37 Christian Grothoff Resolution open => fixed
2018-01-29 17:37 Christian Grothoff Fixed in Version => 0.9.59
2018-01-29 17:37 Christian Grothoff Target Version => 0.9.59
2018-11-06 19:45 Christian Grothoff Status resolved => closed
2021-09-02 17:54 Christian Grothoff Changeset attached => libmicrohttpd master 2dd1b43e
2021-09-02 17:54 Christian Grothoff Note Added: 0018182