Re: [libmicrohttpd] [PATCH]: MHD_connection_update_event_loop_info sends INTERNAL_ERROR for suspended connections

[Evgeny Grin]

On 16.03.2017 16:57, Vitaliy T wrote:
> Hi,
> 
> On 16 March 2017 at 00:29, Vitaliy T <address@hidden> wrote:
>> On 15 March 2017 at 23:54, Evgeny Grin <address@hidden> wrote:
>>> That's why I asked about user code example. I can't test what we trying
>>> to fix.
>>
>> I understand you wish clearly. I will provide the code tomorrow.
> 
> Sources: https://github.com/gh0stwizard/x11mirror-server
> 
> Build:
> 
> # define the location where MHD installed
> export PKG_CONFIG_PATH=/path/to/lib/pkgconfig
> export LD_LIBRARY_PATH=/path/to/lib
> make

[skip]

> 
> About latest changes in git master, commit
> de8217492bec2f08c985fd1a871fe2e6a5b753a6.
> 
> 1. MHD_connection_update_event_loop_info() stills sends INTERNAL_ERROR
> for a suspended connection.
> 2. MHD_resume_connection() does not update connection->last_activity
> 
> With the patch below I see that everything works as I expect (I may be
> wrong, but...)
> 
> 
> diff --git a/src/microhttpd/connection.c b/src/microhttpd/connection.c
> index 74238a1b..697c580d 100644
> --- a/src/microhttpd/connection.c
> +++ b/src/microhttpd/connection.c
> @@ -3217,6 +3217,11 @@ MHD_connection_handle_idle (struct
> MHD_Connection *connection)
>            return MHD_YES;
>          }
>      }
> +    else
> +    {
> +          connection->in_idle = false;
> +          return MHD_YES;
> +    }
>    MHD_connection_update_event_loop_info (connection);
>    ret = MHD_YES;
>  #ifdef EPOLL_SUPPORT
> diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
> index b78aabb0..e93f3dc8 100644
> --- a/src/microhttpd/daemon.c
> +++ b/src/microhttpd/daemon.c
> @@ -2793,6 +2793,7 @@ MHD_resume_connection (struct MHD_Connection 
> *connection)
>    MHD_mutex_lock_chk_ (&daemon->cleanup_connection_mutex);
>    connection->resuming = true;
>    daemon->resuming = true;
> +  connection->last_activity = MHD_monotonic_sec_counter();
>    MHD_mutex_unlock_chk_ (&daemon->cleanup_connection_mutex);
>    if ( (MHD_ITC_IS_VALID_(daemon->itc)) &&
>         (! MHD_itc_activate_ (daemon->itc, "r")) )
> 
> 
> I hope it will help you.

It's hard to test with such huge example.
Could you provide minimal example? Which will fail, but as small as
reasonably possible.

However, I take a look on your code. You resumed connection and
immediately updated timeout. Seems that MHD functions did update of
internal double-linked list in thread-unsafe way.

I partially rewrote code for update of activity, fixed reset timeout
timer on resume.
Could you try an updated master branch?

-- 
Best Wishes,
Evgeny Grin