Re: [libmicrohttpd] upgrading connection

[José Bollo]

Le Wed, 20 Apr 2016 20:42:48 +0300,
Evgeny Grin <address@hidden> a écrit :

> 
> 
> 
> -------- Forwarded Message --------
> Subject: Re: [libmicrohttpd] upgrading connection
> Date: Wed, 20 Apr 2016 19:48:53 +0300
> From: Evgeny Grin <address@hidden>
> To: address@hidden
> 
> 
> 
> On 20.04.2016 18:54, José Bollo wrote:
> > Le mercredi 20 avril 2016 à 18:03 +0300, Evgeny Grin a écrit :
> 
> >> We are collecting ideas how to support "upgrade" in
> >> the best way, you can see some preliminary drafts in disabled part
> >> of microhttpd.h (look for "MHD_UpgradeAction").
> > 
> > Seen. Well... I don't fully understand the design at first look. In
> > particular, the comment of MHD_UpgradeHandler write things about
> > 'data_in/out' but I don't understand what it refers to.
> 
> Give us more ideas, we are open.
> 
> >> Not sure that MHD_suspend_connection()/MHD_resume_connection() are
> >> designed to be used in such way. MHD keeps internal stage of
> >> connection processing and it's not updated after
> >> MHD_resume_connection(). How did you obtain socket FD and when you
> >> are calling MHD_resume_connection()?
> > 
> > I use on suspend (not resume)
> > MHD_get_connection_info(connection,MHD_CONNECTION_INFO_CONNECTION_FD)->connect_fd
> 
> I suppose that you check return value for NULL and used this
> simplified form only to illustrate idea. ;)

ohoho does it means that MHD_get_connection_info could return NULL even
when all is parameters are valid?

> Anyway, if you are not using MHD_resume_connection(), you're leaking
> resources. For every new connection MHD allocate new internal
> structure, which are freed after connection is closed. If you never
> invoke MHD_resume_connection() then connection-specific structure is
> kept forever, even if you close socket in your application.

I invoke resume when the websocket terminates. I think that I have no
leak on the paper but the code isn't terminated and this precise part
is to be done.

> > I configure MHD to use EPOLL
> > 
> > The effect of suspending is that the fd is removed from the file set
> > using  EPOLL_CTL_DEL. This works very well (with the patch) because
> > the buffers are clean. I guess that is will always be the case
> > but...
> As I said, you have resources leak.
> May be we will extend API to allow "detach" of connection FD, but
> currently API frozen until may.
> 
> > Anyway the connection must be kept alive. No?
> Right, handling of "connection: " header must be improved.

May I dream that the minimal patch that removed the test to upgrade
in keep_alive could be accepted?

I didn't run any of the test QA of MHD... So I can not imagine what's
implied.

Best regards
José Bollo

pgpRwjSsxD_9u.pgp
Description: OpenPGP digital signature