Hello Christian,
Thank you for the advise. I had not thought about that before but there
is surely a need for a side channel notification in my case. I have
to check...
The dream for me that it could be blindly integrated in LMHD translates
perhaps to a nightmare for you... I let you mind about that, extend or
not the TODO list, and return to my code.
Thank you again for your work.
Best regards
José
On Wed, 6 Jan 2021 20:45:21 +0100
Christian Grothoff <groth...@gnunet.org> wrote:
> Hi Jose,
>
> Yes, we should indeed return a timeout of 0 in this case. I've
> implemented that in df6124f7..b1691240.
>
> However, please note that I would still strongly advise the use of a
> signalling channel, as your
> approach only works in limited cases where your
> closing of the connection is done within the scope of the MHD_run()
> operation. The moment you actually do such operations later based on
> other things in your external event loop, you will need some kind of
> signalling.
>
> Happy hacking!
>
> Christian
>
> On 1/5/21 10:17 AM, José Bollo wrote:
> > On Sun, 3 Jan 2021 14:22:25 +0100
> > Christian Grothoff <groth...@gnunet.org> wrote:
> >
> >> Hi Jose,
> >>
> >> I've figured out the issue. The bug is actually in your code.
> >
> > Hi Christian,
> >
> > Thank you for having checked that issue.
> >
> >> The problem is that you are using an _external_ event loop, and if
> >> you do so, you are responsible for re-triggering MHD_run() if you
> >> do anything that might change the state of an MHD connection.
> >> This (mostly) applies to MHD_resume_connection() -- but also to
> >> closing an upgraded connection.
> >>
> >> Without this modification, MHD does close the connection on the
> >> _next_ request, so if you would connect with another client, MHD
> >> will then close the connection.
> >
> > Yes, the next request closes the connection and unlocks the first
> > client. But this is not a solution, obviously.
> >
> >> This is the only way to fix this, as when MHD is not 'run', it
> >> cannot act. And if you are controlling the external event loop,
> >> you are responsible for triggering MHD 'when necessary'.
> >>
> >> I've attached a modified version of your code that adds the
> >> necessary signalling logic to your poll()-loop. (Using eventfd();
> >> non-portable. Use a pipe() to do this in a more portable way.)
> >
> > I have checked your rewrite. (A side note, reformatting code is not
> > fair when using tools for a quick search of differences.)
> >
> > Before accepting your conclusion, I'd like to argument a little the
> > reason why I'm still thinking that something can be done in your
> > code.
> >
> > The main loop i submitted is:
> >
> > while (poll(pfd, 1, -1) == 1) {
> > do {
> > MHD_run(d);
> > }
> > while(
> > MHD_get_timeout(d, &to) == MHD_YES
> > && to == 0);
> > }
> >
> > The close is done in a user callback invoked within MHD_run. Then
> > in my opinion, the function MHD_get_timeout should return a timeout
> > of zero because there is no time to wait before the next action:
> > closing something.
> >
> > If you accept that idea, it simplifies my code and probably avoid
> > further questions if someone also integrates an external polling
> > mechanism.
> >
> > Otherwise, lets go for handling specifically the squared corner
> > case of the wheel.
> >
> > Hopefully I convinced you.
> > Best regards
> > José
> >
> >>
> >>
> >> Happy hacking!
> >>
> >> Christian
> >>
> >> On 12/10/20 4:08 PM, José Bollo wrote:
> >>> Hello,
> >>>
> >>> My code uses LMHD embedded with its EPOLL mechanism. Part of that
> >>> code deals with upgrading to websocket. It then call somewhere:
> >>>
> >>> response = MHD_create_response_for_upgrade(
> >>> upgrade_to_websocket, memo);
> >>>
> >>> and the callback function upgrade_to_websocket looks as here
> >>> below:
> >>>
> >>> void upgrade_to_websocket(
> >>> void *cls,
> >>> struct MHD_Connection *connection,
> >>> void *con_cls,
> >>> const char *extra_in,
> >>> size_t extra_in_size,
> >>> MHD_socket sock,
> >>> struct MHD_UpgradeResponseHandle *urh
> >>> ) {
> >>> struct memo *memo = cls;
> >>> struct ws *ws = ws_create(sock, memo, close_websocket, urh);
> >>> if (ws == NULL) close_websocket(urh);
> >>> }
> >>>
> >>> void close_websocket(struct MHD_UpgradeResponseHandle *urh) {
> >>> MHD_upgrade_action (urh, MHD_UPGRADE_ACTION_CLOSE);
> >>> }
> >>>
> >>> Thank you for your attention until here. So far, so good.
> >>>
> >>> The issue now: when the functiuon ws_create returns NULL, the
> >>> program returns to some polling and wait for an events BUT DOES
> >>> NOT CLOSE THE SOCKET, leading to starvation of the client.
> >>>
> >>> I guess that calling some function after calling
> >>> MHD_upgrade_action (urh, MHD_UPGRADE_ACTION_CLOSE) could unlock
> >>> the situation by performing correct close. Though the called
> >>> function should not be MHD_run because it dispatch events, what
> >>> is not expected here.
> >>>
> >>> I join a sample demo. When I connect on websocket on it, the
> >>> client starves. I recorded and joined the output of strace.
> >>>
> >>> Best regards
> >>> José Bollo
> >>>
> >>>
> >
>
