Re: [libmicrohttpd] Busy waiting up to one second using connection timeout

[Evgeny Grin]

Please see my comments below.

On 12.08.2021 12:36, Irion, Alexander wrote:
> I would say that the timer granularity and the fix are two different things, 
> even if granularity would be finer, the checking condition like before was 
> wrong and would lead to unnecessary busy cycles.

That's correct in part about busy cycles. Thanks for the report.
But without changing resolutions of the timer, the proper fix would be 
changing calculation of the value returned by MHD_get_timeout().

> Is a timeout of only one second a real use case? 

Three seconds timeout is more realistic use-case. With this patch it is 
changed to 2-3 seconds.
The condition "greater" (instead of "greater or equal") was used by 
purpose: with limited granularity this is the only way to make 
connection expire *after* defined timeout value, not "after or just 
before" (as with this patch).

> In any way, your example below would result in a returned timeout value of 
> 1000ms, so you would sleep 1s into the external poll, before calling MHD_run() 
> again. So, in fact it would be closed at 51.999. However, yes - when a new 
> event would come in shortly after 50.999 this would trigger the connection 
> close earlier.

That's the case. If more than one connection is served by the same 
server, external poll could be triggered much earlier than one second. 
If many connections are served the chances are quite high that something 
happen within 1/100 of second which trigger MHD_run() which trigger 
immediate connection expiration with this patch.

If user sets timeout value to 10 second, user expects that connection 
would not be closed before 10 seconds of inactivity. As activity newer 
happen at xx.0000 moments, connection will expire (almost) always before 
defined timeout (if more than one connection is served) with this patch.

--
Evgeny

> The report itself is correct, thanks for this!
>
> However while the patch fixed one problem, it creates another one.
> The problem connected to low granularity (rounded to one second) of the
> MHD's internal timer. If connection timeout is set to one second (for
> example) then connection may expire at any moment up to one second with
> the patch applied.
> A full example: if activity is recorded at 50.999 seconds, which is
> rounded to 50 internally, then connection with 1 second timeout can be
> closed at 51.0, which is not what is expected by user.
>
> The proper solution would be increase of the resolution of the internal
> MHD timer.
> I'll handle it.
>
> --
> Evgeny
>
> On 11.08.2021 23:39, Christian Grothoff wrote:
> > Thanks for the detailed report, the patch looks good as-is, applied to
> > master as
> >
> > f5d387df7e7fb8de1a5dd0739ddb83a8b19fe64b
> >
> > Happy hacking!
> >
> > Christian
> >
> > On 8/11/21 6:02 PM, Irion, Alexander wrote:
> > > Hello!
> > >
> > > I discovered a problem with closing connections, when
> > > MHD_OPTION_CONNECTION_TIMEOUT is used. When I set the timeout option to
> > > 3 seconds and connect with a browser, I get a timeout value from
> > > MHD_get_timeout() of 3000 ms returned. I then sleep the desired time and
> > > call MHD_run again(). Then, MHD_get_timeout() returns repetitive a
> > > timeout of 0 which causes thousands of calls to MHD_run(), taking up
> > > several hundred milliseconds and burning CPU time.
> > >
> > > I use an external event loop with MHD_DAEMON_INFO_EPOLL_FD.
> > >
> > > As reason I found that the condition  timeout <
> > > (MHD_monotonic_sec_counter () - connection->last_activity) will not be
> > > true, after waiting the timeout time. The right side will be equal to
> > > timeout, so the connection is not closed, MHD_get_timeout() continues to
> > > return a timeout value of 0, and this runs repetitively in a loop for
> > > some thousand iterations, until the time jumps to the next second.
> > >
> > > In the attachment please find a small patch for fixing this issue. It's
> > > just turning the < into a <=.
>
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

OpenPGP_signature
Description: OpenPGP digital signature