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
>>>
>>>
>
signature.asc
Description: OpenPGP digital signature
