From 8e1f21d47b37e1922bd1c88bd69378dc5e016acc Mon Sep 17 00:00:00 2001 From: Matteo Scaramuccia Date: Sun, 22 Dec 2013 15:27:54 +0100 Subject: [PATCH] MDL-43483 mod_chat: Fixed race condition issues, including not useful logs. --- mod/chat/chatd.php | 51 +++++++++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/mod/chat/chatd.php b/mod/chat/chatd.php index 69b6a681d85..37462c0e4ec 100644 --- a/mod/chat/chatd.php +++ b/mod/chat/chatd.php @@ -130,7 +130,10 @@ class ChatDaemon { foreach ($chatroom['users'] as $sessionid => $userid) { // We will be polling each user as required. $this->trace('...shall we poll '.$sessionid.'?'); - if ($this->sets_info[$sessionid]['chatuser']->lastmessageping < $this->_last_idle_poll) { + if (!empty($this->sets_info[$sessionid]) && isset($this->sets_info[$sessionid]['chatuser']) && + // Having tried to exclude race conditions as already done in user_lazy_update() + // please do the real job by checking the last poll. + ($this->sets_info[$sessionid]['chatuser']->lastmessageping < $this->_last_idle_poll)) { $this->trace('YES!'); // This user hasn't been polled since his last message. $result = $this->write_data($this->conn_sets[$sessionid][CHAT_CONNECTION_CHANNEL], ''); @@ -209,6 +212,11 @@ class ChatDaemon { return false; } + // Does promote_final() already finish its job? + if (!isset($this->sets_info[$sessionid]['lastinfocommit'])) { + return false; + } + $now = time(); // We 'll be cheating a little, and NOT updating the record data as @@ -231,6 +239,7 @@ class ChatDaemon { $timenow = time(); if (empty($str)) { + $str = new stdClass(); $str->idle = get_string("idle", "chat"); $str->beep = get_string("beep", "chat"); $str->day = get_string("day"); @@ -700,7 +709,12 @@ EOD; $monitor = array(); if (!empty($this->conn_ufo)) { foreach ($this->conn_ufo as $ufoid => $ufo) { - $monitor[$ufoid] = $ufo->handle; + // Avoid socket_select() warnings by preventing the check over invalid resources. + if (is_resource($ufo->handle)) { + $monitor[$ufoid] = $ufo->handle; + } else { + $this->dismiss_ufo($ufo->handle, false); + } } } @@ -735,21 +749,24 @@ EOD; // Simply give them the message. $output = chat_format_message_manually($message, $info['courseid'], $sender, $info['user']); - $this->trace('Delivering message "'.$output->text.'" to '.$this->conn_sets[$sessionid][CHAT_CONNECTION_CHANNEL]); + if ($output !== false) { + $this->trace('Delivering message "'.$output->text.'" to ' . + $this->conn_sets[$sessionid][CHAT_CONNECTION_CHANNEL]); - if ($output->beep) { - $this->write_data($this->conn_sets[$sessionid][CHAT_CONNECTION_CHANNEL], - ''); - } + if ($output->beep) { + $this->write_data($this->conn_sets[$sessionid][CHAT_CONNECTION_CHANNEL], + ''); + } - if ($info['quirks'] & QUIRK_CHUNK_UPDATE) { - $output->html .= $GLOBALS['CHAT_DUMMY_DATA']; - $output->html .= $GLOBALS['CHAT_DUMMY_DATA']; - $output->html .= $GLOBALS['CHAT_DUMMY_DATA']; - } + if ($info['quirks'] & QUIRK_CHUNK_UPDATE) { + $output->html .= $GLOBALS['CHAT_DUMMY_DATA']; + $output->html .= $GLOBALS['CHAT_DUMMY_DATA']; + $output->html .= $GLOBALS['CHAT_DUMMY_DATA']; + } - if (!$this->write_data($this->conn_sets[$sessionid][CHAT_CONNECTION_CHANNEL], $output->html)) { - $this->disconnect_session($sessionid); + if (!$this->write_data($this->conn_sets[$sessionid][CHAT_CONNECTION_CHANNEL], $output->html)) { + $this->disconnect_session($sessionid); + } } } } @@ -950,6 +967,12 @@ while (true) { continue; } + // Ignore desktop browser fake "favorite icon" requests. + if (strpos($data, 'GET /favicon.ico HTTP') === 0) { + // Known malformed data, drop it without any further notice. + continue; + } + if (!preg_match('/win=(chat|users|message|beep).*&chat_sid=([a-zA-Z0-9]*) HTTP/', $data, $info)) { // Malformed data. $daemon->trace('UFO with '.$handle.': Request with malformed data; connection closed', E_USER_WARNING);