\"General\" Chat Room Stop Accepting Messages

[clifton11221]

Everything was fine... Then nobody could type any messages in the "General" chat room. All "private" chats are going through... but it won't accept anything in the "General" chat.

Any ideas on how to fix it... I'm using beta 11?

[clifton11221]

I do not know what caused the problem or how to stop it from happening.

However, a temp fix is to delete all of the chat, cache & log files:

/data/private/logs
/data/private/chat
/data/private/cache


If someone know how to resolve this issue please let me know.

[maxeii]

I have the same problem.

because file size???

When I running some day. it's must be trouble.:XD

[lackattack]

I have this problem too and every few days we have to delete those files. I even wrote a special admin command so that my moderators can /identify and fix it!

As you can see from my configuration below I have two chat rooms. Somtimes one freezes, sometimes it's the other one.

Code: Select all

require_once dirname(__FILE__)."/src/phpfreechat.class.php";
$params = array();
$params["title"] = "Live Chat";
$params["nick"] = $_COOKIE['u'];
$params["serverid"] = md5(__FILE__); // calculate a unique id for this chat
$params["data_private_path"] = "/dev/shm/conquerchat";
$params["theme"] = "phpbb2";
$params['channels'] = array('Callouts','Social');
$params["frozen_nick"] = true;
$params["max_nick_len"] = 16;
$params["refresh_delay"] = 3000; // 2000ms = 2s
$params["height"] = "350px";
$params['admins'] = array('lackattack'=>'','AndyDufresne'=>'','wicked'=>'','moz976'=>'','AK_iceman'=>'','mrdexter'=>'','Fireside Poet'=>'','king achilles'=>'');
$params["time_offset"] = (int)$_COOKIE['o'];
$chat = new phpFreeChat( $params );

This time the "Social" room froze and I looked into the temp files to find a clue. In logs/.../chat.log I discovered that messages typed in Social were still being logged even though they don't display:

Code: Select all

ch_Callouts   20/07/2007   07:02:50   qeee1   hey kidtwist
ch_Social   20/07/2007   07:02:53   lackattack   testing
ch_Callouts   20/07/2007   07:03:07   KidTwist   thought you were quitting?

In chat/s_.../channelid-to-msgid/ch_Social/lastmsgid was 509 and 509 is indeed the file with the latest timestamp in chat/s_.../channelid-to-msg/ch_Social/. If I view 509 its contents match the last Social chat message in chat.log. So everything looks good.

The only this that is suspect is the message files in chat/s_.../channelid-to-msg/ch_Social/. They go from 22078 (1:37am) through 22117 (2:08am) and then jump to 470 (9:54am) through 509 (10:08am).

I hope this can help find the problem. Next time it happens I will turn on debug

[King Moonraiser]

Just wanted to say this is happening for me, too. Since I have shell access to my server, I made a script that just reinstalls the whole darn chat software with my mods. I hope your findings will lead to a bug fix.

[phpfreechat]

I think this is a bug in the file container.
Index files are used to know the last posted messages on each channel. These files contains integers and each time a new messages is posted the integer value is increased.
Sometimes this index file is corrupted and the integer comes back to 0 whereas the last message is not 0 !
So as long as the index integer value is smaller than the real last message value, nothing will be shown.

We have to find a way to avoid the corruption of this index file.
If someone know php and file locking maybe he could help us. Here is the code :

Code: Select all

  /**
   * Returns the last message id
   * Notice: the default file container just returns the messages.index file content
   * @param $chan if NULL then read if from the server, otherwise read if from the given channel
   * @return int is the last posted message id
   */
  function getLastId($chan)
  {
    if ($chan == NULL) $chan = 'SERVER';

    $lastmsgid = $this->getMeta("channelid-to-msgid", $this->encode($chan), 'lastmsgid', true);
    if (count($lastmsgid["value"]) == 0)
      $lastmsgid = 0;
    else
      $lastmsgid = $lastmsgid["value"][0];
    return $lastmsgid;
  }


  /**
   * Return a unique id. Each time this function is called, the last id is incremented.
   * used internaly
   * @private
   */
  function _requestMsgId($chan)
  {
    if ($chan == NULL) $chan = 'SERVER';

    $lastmsgid = $this->getLastId($chan);
    $lastmsgid++;
    $this->setMeta("channelid-to-msgid", $this->encode($chan), 'lastmsgid', $lastmsgid);

    return $lastmsgid;
  }

Code: Select all

  function setMeta($group, $subgroup, $leaf, $leafvalue = NULL)
  {
    $c =& pfcGlobalConfig::Instance();

    if ($c->debug)
      file_put_contents("/tmp/debug", "nsetMeta(".$group.",".$subgroup.",".$leaf.",".$leafvalue.")", FILE_APPEND);

    // create directories
    $dir_base = $c->container_cfg_server_dir;
    $dir = $dir_base.'/'.$group.'/'.$subgroup;
    if (!is_dir($dir)) mkdir_r($dir);

    // create or replace metadata file
    $leaffilename = $dir."/".$leaf;
    $leafexists = file_exists($leaffilename);
    if ($leafvalue == NULL)
    {
      if (file_exists($leaffilename) &&
          filesize($leaffilename)>0) unlink($leaffilename);
      touch($leaffilename);
    }
    else
    {
      flock_put_contents($leaffilename, $leafvalue);
    }

    // store the value in the memory cache
    //@todo
    //    $this->_meta[$enc_type][$enc_subtype][$enc_key] = $value;

    if ($leafexists)
      return 1; // value overwritten
    else
      return 0; // value created
  }


  function getMeta($group, $subgroup = null, $leaf = null, $withleafvalue = false)
  {
    $c =& pfcGlobalConfig::Instance();
    if ($c->debug)
      file_put_contents("/tmp/debug", "ngetMeta(".$group.",".$subgroup.",".$leaf.",".$withleafvalue.")", FILE_APPEND);

    // read data from metadata file
    $ret = array();
    $ret["timestamp"] = array();
    $ret["value"]     = array();
    $dir_base = $c->container_cfg_server_dir;

    $dir = $dir_base.'/'.$group;
    if ($subgroup == NULL)
    {
      if (is_dir($dir))
      {
        $dh = opendir($dir);
        while (false !== ($file = readdir($dh)))
        {
          if ($file == "." || $file == "..") continue; // skip . and .. generic files
          $ret["timestamp"][] = filemtime($dir.'/'.$file);
          $ret["value"][]     = $file;
        }
        closedir($dh);
      }
      return $ret;
    }
    $dir .= '/'.$subgroup;

    if ($leaf == NULL)
    {
      if (is_dir($dir))
      {
        $dh = opendir($dir);
        while (false !== ($file = readdir($dh)))
        {
          if ($file == "." || $file == "..") continue; // skip . and .. generic files
          $ret["timestamp"][] = filemtime($dir.'/'.$file);
          $ret["value"][]     = $file;
        }
        closedir($dh);
      }

      return $ret;
    }

    $leaffilename = $dir."/".$leaf;

    if (!file_exists($leaffilename)) return $ret;
    if ($withleafvalue)
      $ret["value"][] = flock_get_contents($leaffilename);
    else
      $ret["value"][] = NULL;
    $ret["timestamp"][] = filemtime($leaffilename);

    return $ret;
  }

[King Moonraiser]

Maybe it's a synchronization problem? Does the _requestMsgId function get called by separate threads? What happens if more than one person sends a message at the same exact time?

Instead of keeping an internal counter ($lastmsgid), maybe using time() (number of seconds since the Unix Epoch) would be better? Even if the counter is lost, you're guaranteed that you can restart with a new value that will always be greater.

Code: Select all

  function _requestMsgId($chan)
  {
    if ($chan == NULL) $chan = 'SERVER';

    $lastmsgid = time();
    $this->setMeta("channelid-to-msgid", $this->encode($chan), 'lastmsgid', $lastmsgid);

    return $lastmsgid;
  }

[phpfreechat]

Thank you for your suggestion King Moonraiser.
However using time() for lastmsgid should not work in all cases. If two users send a message at the same second then they will get the same id for their messages.

However your idea to use the timestamp to detect possible id corruption could be useful.
Maybe it can help the program to initialize the msgid counter when it is corrupted...

Anyway, I think the cleaner idea is to use file locking to avoid possible file corruptions.
Here is the code I use to read and write in files ... it should have bug but where ?

Code: Select all

/**
 * Almost the Same as file_get_contents except that it uses flock() to lock the file
 */
function flock_get_contents($filename, $mode = "rb")
{
  $data = '';
  if (!file_exists($filename)) return $data;

  $size = filesize($filename);
  if ($size == 0) return $data;

  $fp   = fopen( $filename, $mode );
  if( $fp && flock( $fp, LOCK_SH ) )
  {
    $data = fread( $fp, $size );
    // flock($fp, LOCK_UN); // will be done by fclose
  }
  fclose( $fp );
  return $data;
}

/**
 * Almost the Same as file_put_contents except that it uses flock() to lock the file
 */
function flock_put_contents($filename, $data, $mode = "wb")
{
  $fp   = fopen( $filename, $mode );
  if( $fp )
  {
    if ( flock( $fp, LOCK_EX ) )
    {
      fwrite( $fp, $data );
      // flock($fp, LOCK_UN); // will be done by fclose
    }
    fclose( $fp );
  }
}

[King Moonraiser]

Anyway, I think the cleaner idea is to use file locking to avoid possible file corruptions.
Here is the code I use to read and write in files ... it should have bug but where ?

I think the "filesize" may return zero if another thread is writing to the file with a lock on. You should move the check for zero file length AFTER the file is locked for reading. By doing this, we are guaranteed that the file is done being written. I added clearstatcache() to make sure PHP isn't returning old size information.

Do you even need to check the filesize if the file is opened and locked? If it was zero file length, wouldn't $data be "" anyway?

Code: Select all

/**
 * Almost the Same as file_get_contents except that it uses flock() to lock the file
 */
function flock_get_contents($filename, $mode = "rb")
{
  $data = '';
  // if (!file_exists($filename)) return $data;

  $fp   = fopen( $filename, $mode );
  if( $fp && flock( $fp, LOCK_SH ) )
  {
    clearstatcache();
    $size = filesize($filename);
    if ($size > 0)
    {
      $data = fread( $fp, $size );
    }
    // flock($fp, LOCK_UN); // will be done by fclose
  }
  fclose( $fp );
  return $data;
}

UPDATE: I commented out the check for file_exists, too. I did a search for "fopen" throughout all of the php files and I noticed flock isn't being used in some cases. I don't know if those reads or writes are related to this problem, but they ought to be updated to use file locks, too.

[phpfreechat]

I think the "filesize" may return zero if another thread is writing to the file with a lock on.

Did you check this point with a real test ?
If it is the case, then I think you found a big potential problem.
Moreover you are right when reading the code again, the filesize test should be useless ... I will just remove it, it should optimize the code.

[King Moonraiser]

I haven't attempted to create a test bed. However, I've implemented the change on my chat software. The problem would usually occur within 2 days. It's not scientific, but it's the best I can do right now.

What about those other parts of the PHP files that don't implement file locking?

UPDATE: I wrote two test php scripts that confirm this bug (although it may not be the only bug )

The first script can be used to ensure your PHP installation is properly locking files for write. If you open up two browser windows and try to open this file at the same time, one should complete 5 seconds after the other. Both instances should be able to get file locks. If not, then your php server's file locking is not functioning properly.

Code: Select all

<?php
$starttime = time();
$date = date('h:i:s',$starttime);
echo 'start:'.$date.'<br />';

$handle = fopen('test.txt','wb');
if($handle){
    $flock = flock($handle,LOCK_EX);
    echo '$flock '; var_dump($flock);echo '<br />';
    if(!$flock){
        echo 'no lock for me<br />';
    }else{
        echo 'got a lock<br />';
        fwrite( $handle, ' start '.$date.' ' );
        sleep(5); //assume the whole writing process takes 5 seconds
        fwrite( $handle, ' end '.$date."n" );
    }
    fclose($handle);
}
echo 'end: this took '.(time()-$starttime).' seconds';
?>

The second script simply prints out the file size. I put it in a for loop to output the value 20 times in case you'd like to other kinds of read/write testing. If you open the first script in a browser window and the second one in another browser window (refresh the second one periodically), you can see that the output to the file is first 16 bytes, then after 5 seconds, jumps to 30. Therefore, if your read script is relying on the filesize obtained during a write, your data will be truncated.

Code: Select all

<?php
$starttime = time();
$date = date('h:i:s',$starttime);
echo 'start:'.$date.'<br />';

for ($i = 0; $i < 20; $i++)
{
  echo 'File size: '; var_dump(filesize('test.txt'));echo '<br />';
}
echo 'end: this took '.(time()-$starttime).' seconds';
?>

Now here's the improved version of the second script. It reads the file size only when a read lock is available. This time it waits until the write script is done and then gets the file size.

Code: Select all

<?php
$starttime = time();
$date = date('h:i:s',$starttime);
echo 'start:'.$date.'<br />';

$handle = fopen('test.txt','rb');
if($handle)
{
  $flock = flock($handle,LOCK_SH);
  echo '$flock '; var_dump($flock);echo '<br />';
  if(!$flock){
      echo 'no lock for me<br />';
  }
  else
  {
    echo 'got a lock<br />';
    for ($i = 0; $i < 20; $i++)
    {
      echo 'File size: '; var_dump(filesize('test.txt'));echo '<br />';
    }
  }
  fclose($handle);
}

echo 'end: this took '.(time()-$starttime).' secconds';
?>

If you do two consecutive writes and then a read, the read will get its result after the first read. Wild, eh?

BTW, the only code that uses the flock functions are the getMeta and setMeta functions. Why do you create the flock functions instead of using "file_put_contents()" and updating the "file_set_contents()" to use flock? As long as the data you are trying to read or write is stored in a scalar, they ought to work.

EDIT: I made a typo. I meant "file_get_contents()" instead of "file_set_contents()." I explain myself better in the reply below.

[phpfreechat]

I don't understand your last paragraph about "file_put_contents()" and "file_set_contents()", could you reformulate it ?

Concerning the optimization and bug fix your suggested, I commited it in svn, here is the new code :

Code: Select all

function flock_get_contents($filename, $mode = "rb")
{
  $data = '';
  $fp = fopen( $filename, $mode );
  if( $fp && flock( $fp, LOCK_SH ) )
  {
    clearstatcache();
    $size = filesize($filename);
    if ($size > 0)
      $data = fread( $fp, $size );
    // flock($fp, LOCK_UN); // will be done by fclose
  }
  fclose( $fp );
  return $data;
}

Do you agree with it ?

[King Moonraiser]

Yes, I do. However...

I was suggesting to eliminate the "flock_get_contents()" and "flock_put_contents()" functions altogether. Why not use the existing "file_put_contents()" (the one you have in pfc_tools.php is directly from the PEAR compatibility library) since it already uses file locking? I also suggest you replace the "file_get_contents()" function with a patched version of the "file_get_contents()" function also from the PEAR library. I downloaded the 1.5.0 PEAR compatibility library, and based my patch on it:

Code: Select all

if (!defined('LOCK_SH')) {
    define('LOCK_SH', 1);
}

/**
 * Replace file_get_contents()
 *
 * @category    PHP
 * @package     PHP_Compat
 * @link        http://php.net/function.file_get_contents
 * @author      Aidan Lister <aidan@php.net>
 * @version     $Revision: 1.21 $
 * @internal    resource_context is not supported
 * @since       PHP 5
 * @require     PHP 4.0.0 (user_error)
 */

function file_get_contents_flock($filename, $incpath = false, $resource_context = null)
{
    if (false === $fh = fopen($filename, 'rb', $incpath)) {
        user_error('file_get_contents() failed to open stream: No such file or directory',
            E_USER_WARNING);
        return false;
    }

    // Attempt to get a shared (read) lock
    if (!flock($fh, LOCK_SH)) {
      return false;
    }

    clearstatcache();
    if ($fsize = @filesize($filename)) {
        $data = fread($fh, $fsize);
    } else {
        $data = '';
        while (!feof($fh)) {
            $data .= fread($fh, 8192);
        }
    }

    fclose($fh);
    return $data;
}

The "file_get_contents_flock()" function above should be used every place the "file_get_contents()" function is called.

By using the "file_get_contents_flock()" and existing "file_put_contents()" consistently throughout the php code, you should be assured of file reads and writes not getting corrupted. Ideally, you won't need to use fopen, flock, fclose, etc. anywhere else.

The next area to examine is your setMeta and getMeta functions. You're performing some operations there that delete and recreate zero-length files using unlink and touch. If you need to zero-out a file, perhaps it would be safer to simply overwrite the file with no data? I've been trying to find out how unlink, touch, file_exists, etc. work in a multi-threaded environment. I haven't found much information on this specific topic. Therefore, I recommend avoiding these operations whenever possible.

[phpfreechat]

As I based all the file container on flock_get_contents and flock_put_contents, I think it's useless to rename these function to "file_get_contents_flock" and "file_put_contents".
The modified flock_get_contents should work now so why not just keep it ?

I'm waiting for your review on setMeta and getMeta and I hope you will find some better way to access data on multi-threaded env. Moreover I propose your to join the sourceforge dev team to be able to submit your patchs directly on the subversion.

[King Moonraiser]

I have all of the patches ready. I'm ready to commit to Sourceforge. How do I become a member of the dev team?