[jira] [Comment Edited] (GUACAMOLE-608) Daemon process may encounter dead loop

["Changkun Ou (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GUACAMOLE-608?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16577979#comment-16577979 ] 

Changkun Ou edited comment on GUACAMOLE-608 at 8/13/18 9:01 AM:
----------------------------------------------------------------

I found something weird.

In {{guac_vnc_client_thread}} ([https://github.com/apache/guacamole-server/blob/0062f61d67163537929d699541356cdce3b36519/src/protocols/vnc/vnc.c#L166]), a loop [https://github.com/apache/guacamole-server/blob/0062f61d67163537929d699541356cdce3b36519/src/protocols/vnc/vnc.c#L334], checks {{wait_result = guac_vnc_wait_for_messages}} on two different condition: {{wait_result > 0}} ([https://github.com/apache/guacamole-server/blob/0062f61d67163537929d699541356cdce3b36519/src/protocols/vnc/vnc.c#L339]) and {{wait_result < 0}} ([https://github.com/apache/guacamole-server/blob/0062f61d67163537929d699541356cdce3b36519/src/protocols/vnc/vnc.c#L390]).

But the result of {{wait_result == 0}} is not verified.

I observed the dead loop process has no log output of {{Internal VNC client disconnected}}, but {{Last user of connection disconnected}} can be observed in log outputs. When {{guacd_user_thread}} exit, the {{guac_client}} is not going to {{free}}, thus the loop keeps running. The result of {{guac_vnc_wait_for_messages}} rely on {{libvncserver}} {{WaitForMessage}}, but the return value of {{WaitForMessage}} is possible to be zero because of {{select}} call: [https://github.com/LibVNC/libvncserver/blob/6814e946e0afed20a6ef0f45a9bcbfeda2e77706/libvncclient/sockets.c#L835]

I don't know whether this is relevant to the issue (I use VNC in practice), what do you think? Is this intentional or a missing? Will it cause a tight loop if {{wait_result}} keep 0 when a {{guac_user_thread}} exit?

 

BTW: there are some constraints that I couldn't patch a gdb debugging version to the production environment. I have to do "white" testing...


was (Author: changkun):
I found something weird.

In {{guac_vnc_client_thread}} (https://github.com/apache/guacamole-server/blob/0062f61d67163537929d699541356cdce3b36519/src/protocols/vnc/vnc.c#L166), a loop https://github.com/apache/guacamole-server/blob/0062f61d67163537929d699541356cdce3b36519/src/protocols/vnc/vnc.c#L334, checks {{wait_result = guac_vnc_wait_for_messages}} on two different condition: {{wait_result > 0}} (https://github.com/apache/guacamole-server/blob/0062f61d67163537929d699541356cdce3b36519/src/protocols/vnc/vnc.c#L339) and {{wait_result < 0}} (https://github.com/apache/guacamole-server/blob/0062f61d67163537929d699541356cdce3b36519/src/protocols/vnc/vnc.c#L390).

But the result of {{wait_result == 0}} is not verified. 

I observed the dead loop process has no log output of {{Internal VNC client disconnected}}, but {{Last user of connection disconnected}} can be observed in log outputs. When {{guacd_user_thread}} exit, the {{guac_client}} is not going to {{free}}, thus the loop keeps running. The result of {{guac_vnc_wait_for_messages}} rely on {{libvncserver}} {{WaitForMessage}}, but the return value of {{WaitForMessage}} is possible to be zero because of {{select}} call: https://github.com/LibVNC/libvncserver/blob/6814e946e0afed20a6ef0f45a9bcbfeda2e77706/libvncclient/sockets.c#L835

I don't know whether this is relevant to the issue (I use VNC in practice), what do you think? Is this intentional or a missing? Will it cause a tight loop if {{wait_result}} keep 0 when a {{guac_user_thread}} exit?

> Daemon process may encounter dead loop
> --------------------------------------
>
>                 Key: GUACAMOLE-608
>                 URL: https://issues.apache.org/jira/browse/GUACAMOLE-608
>             Project: Guacamole
>          Issue Type: Bug
>          Components: guacd
>    Affects Versions: 0.9.14, 1.0.0
>            Reporter: Changkun Ou
>            Priority: Minor
>
> {{guacd}} involves a {{__write_all}} function to write instruction as much as possible, see https://github.com/apache/guacamole-server/blob/7c191d7be0441a1cb64c90ab62d6535f3798eacb/src/guacd/connection.c#L67
> However system call {{write}} may return 0 and set {{errno}}, which is not verified in the function.
> A possible case: {{write}} keeps return 0 and nothing writes to buffer, therefore the daemon process encounters a dead loop, furthermore, it leads CPU rate up to 99%.
> A possible fix is:
> {code:none}
> int written = write(fd, buffer, length);
> if (written < 0 || errno > 0)
>     return -1;
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)