Re: [libmicrohttpd] recv MSG_DONTWAIT and EAGAIN

[Christian Grothoff]

You are right.  MHD_poll should behave differently in combination with 
MHD_USE_THREAD_PER_CONNECTION and not go over the individual connections.  
This also fixes the other issue you mention: once we stop doing that nonsense, 
the 'struct pollfd p[]' won't be an array at all anymore (for the case of 
'use-thread-per-connection' --- which is the case where the per-thread stack 
utilization matters).  So fixing this problem actually addresses both of these 
issues at the same time.  I've put a preliminary patch into SVN as of revision 
14676, but a bit more testing would be good...

Happy hacking,

Christian

On Friday, March 18, 2011 08:35:35 pm Eivind Sarto wrote:
> I think there is a problem with using flags = MHD_USE_THREAD_PER_CONNECTION
> | MHD_USE_POLL
> 
> The MHD_select_thread() calls MHD_poll().  MHD_poll() does not check for
> MHD_USE_THREAD_PER_CONNECTION and proceeds to call all the read handlers
> for all the connections.  And this races against the threads processing
> the current connection. That is probably why recv gets the EAGAIN.  The
> connection thread has already consumed the data when the select thread
> calls the read handler, or the other way around.
> 
> Another note.  MHD_poll() allocates the pollfd on the stack:
>     struct pollfd p[1 + num_connections];
> I was hoping to be able to run libmicrohttpd with a large number of
> connections (and a relatively small thread stack). Maybe it could be
> malloc once and saved in struct MHD_Daemon, or something like that?
> 
> -eivind
> ________________________________________
> From: address@hidden
> address@hidden On Behalf Of Christian
> Grothoff address@hidden Sent: Friday, March 18, 2011 5:30 AM
> To: address@hidden
> Cc: Eivind Sarto
> Subject: Re: [libmicrohttpd] recv MSG_DONTWAIT and EAGAIN
> 
> Given that we defined MSG_DONTWAIT to 0 if it was not defined by the
> platform, clearly the logic was never supposed to depend on non-blocking
> operations. I think the DONTWAIT was added as a "conservative" flag ("just
> in case"), after all, the calls are all guarded by 'select' and 'poll'.
> 
> So this makes the situation here interesting:
> 
> 1) select/poll should only allow us to get to recv if data is ready (and
> note
> 
> that we're using select/poll even with threads). So technically the
> 
> DONTWAIT should not have mattered --- dead code should go, right?
> 
> 2) On the other hand, the DONTWAIT may have prevented worse -- a blocking
> 
> read of the server due to some other error that would stop all
> 
> connections was avoided at the expense of dropping one.
> 
> Clearly neither is good. Furthermore, we got an EAGAIN which, while
> unexpected (see above) it by itself usually not a reason to close a
> connection --- except if our select/poll is broken and we're about to go
> into a busy-waiting spin cycle, maybe then closing the connection is a
> good idea after all?
> 
> Finally, based on your report, the issue is tricky for you to reproduce
> ("timing related"), so I don't have an easy testcase to settle who is to
> blame (after all, theoretically your application could have read data from
> the socket between MHD's select/poll and recv call, or corrupted memory,
> or ... --- not that I'm saying this is even likely, my money is on
> poll+threads still having some deeper trouble that I just cannot find by
> looking at the code).
> 
> Given all this, I've decided to follow your suggestion and remove
> MSG_DONTWAIT (from both recv and send calls) for now, the primary reason
> being that it was never used on *all* platforms and so this should give us
> consistent behavior, if nothing else, and, by design, it should do no
> harm. [SVN 14672]
> 
> I'll keep the code to close connections on EAGAIN as well, after all, that
> should not have happened and now definitively should never happen in the
> first place.
> 
> Naturally, I'd be very interested to hear if someone now suddenly observes
> blocking behavior on a recv call (obviously hard to observe on
> pthread-per-connection...) even more so if they can produce a testcase to
> demonstrate it.
> 
> Happy hacking,
> 
> Christian
> 
> On Wednesday, March 16, 2011 07:25:05 pm Eivind Sarto wrote:
> > I am using libmicrohttpd-0.9.8 for a prototype video streaming
> > application
> > 
> > and I am seeing some unexpected error messages from the library when I
> > run
> > 
> > the application on a very fast server. I think the behavior is timing
> > 
> > related, because it does not occur when I run the same code on a slightly
> > 
> > slower server, or if I enable an MHD_OPTION_URI_LOG_CALLBACK handler on
> > 
> > the fast server.
> > 
> > 
> > 
> > In daemon.c:recv_param_adapter(), RECV(MSG_DONTWAIT) will frequently
> > return
> > 
> > EAGAIN, and this causes connection.c:do_read() to call
> > 
> > connection_close_error().
> > 
> > 
> > 
> > Is this the right logic?
> > 
> > Calling RECV(MSG_DONTWAIT) should expect EAGAIN if there is no data
> > queued
> > 
> > on the socket head. It is not really and error that should close the
> > 
> > connection.
> > 
> > What do you think?
> > 
> > 
> > 
> > I am running the library with flags = MHD_USE_THREAD_PER_CONNECTION |
> > 
> > MHD_USE_POLL, so for debug I decided to try and remove the MSG_DONTWAIT
> > 
> > flag. And the application would then run without any errors.
> > 
> > 
> > 
> > Note: I do not have any control over the client. I am streaming live
> > video
> > 
> > to an Apple handheld device.
> > 
> > 
> > 
> > -eivind