de-merphq is borged. If someone unborgs him, then fullpage chat will start emitting server errors.

The root cause is that unpackVars (used by getVars) incorrectly returns undef instead of an empty hash (reference) for empty settings which causes an error in lines 36 and 37 of talkbar.

Please don't unborg de-merphq (or whoever demerphq replaces that with) until someone can fix this bug.

- tye        

Replies are listed 'Best First'.
Re: getVars bug kills full-page chat
by demerphq (Chancellor) on Dec 27, 2004 at 12:48 UTC

    The following patch should resolve this problem.

    --- orig\Everything.pm 2004-12-27 13:17:23.025625000 +0100 +++ new\Everything.pm 2004-12-27 13:20:39.213125000 +0100 @@ -349,6 +349,7 @@ printLog("Undefined type in unpackVars()"); return "Error undef type"; } elsif ($type eq 'H') { + $vars={}; for (split /\Q$split\E/, $vars_str) { my ($k,$v)= split /!/, $_, 2; for ( $k,$v ) { @@ -364,6 +365,7 @@ $vars->{$k} = $v; } } elsif ($type eq 'A') { + $vars=[]; for (split /\Q$split\E/, $vars_str) { s/~(\w\w)/ chr(hex($1)) /ge; push @$vars, $_ eq 'U'

    And while we are talking about patching PM modules, Id like to know what policy we have with version numbers in the modules. It doesnt seem like we bump the version numbers when we patch, and unless im totally mistaken they are far different from the similarly numbered version from Everything Development. Perhaps we should start a new version sequence? Maybe call ours Version=100.0 or something and then increment from there?

    A last question I have concerns the loging mechansim:

    sub printLog { my $entry = $_[0]; my $time = getTime(); # prefix the date a time on the log entry. $entry = "$time: $entry\n"; if(open(ELOG, ">> $everythingLog")) { print ELOG $entry; close(ELOG); } return 1; }

    It opens and closes the file for every statement!! IMO it should look more like the following (untested) snippet:

    { my $logfile; my $loghandle; use Fcntl qw(:DEFAULT :flock); require POSIX; sub printLog { if (!$loghandle or $logfile ne $everythingLog) { $logfile=$everythingLog; open $loghandle,">>",$logfile or return; } my $time = POSIX::strftime( "%Y-%m-%d %H:%M:%S >", localtime() ); flock( $loghandle, LOCK_EX ) or return; seek( $loghandle, 0, 2 ) or return; # prefix the date and time on the log entry. print $loghandle map("$time: $_\n",@_) or return; flock( $loghandle, LOCK_UN ) or return; return 1; } }
    ---
    demerphq

      I bump version numbers when I patch. If you want to jump to some arbitrary high version number(s) just so those e2 guys know that our stuff is better than theirs, then feel free to apply such a patch. (:

      Leaving the file open complicates rotating the log. I don't know how the log is currently rotated, but for the minor price of writing a log line (which shouldn't take long at all) being about 3 times as slow (according to your benchmarks), I'd rather stick with the open/close cycle so that whatever rotation scheme is in place works.

      Eliminating nearly all of the open/close'ing would be fine if we go with a log rotation scheme that restarts apache after the log file is rotated such that the time during which log lines could go to either file is minimized.

      - tye        

        Ok. I wasnt aware you were bumping version numbers when you applied patches. I didnt notice that it changed when the getVars() stuff changed so I assumed we didn't do it at all. Regarding applying patches to the PM modules, my preference as you know is to leave it to you or theorbtwo. I was just wondering about the policy. :-)

        As for the logging, I think that at some point it would be worthwhile looking into the process and making it lighter load, but it can wait.

        ---
        demerphq

      I think using locking on the logfile will be a bad idea, as it will/could create logfile contention, as the processes will have to wait to commit a line to the log. In theory (that is, tye said so) appending a line to the logfile is atomic and we shouldn't need any locking anyway. Keeping the FH open will be bad mojo though - maybe we should do the ugly dance of seek-to-end, syswrite. But I would like to see arguments/statistics that the opening/closing of files is a major slowdown factor over the locking. Since the multiple lines get spewed into the logfile directly anyway, I don't expect a major speedup, but maybe we could switch the whole stuff to syslogd or some other log aggregator mechanism that is easily available on pairs servers already...

      Patch applied, version numbers bumped, Apaches restarted, user unborg'd, fullpage chat tested. All fixed. Thanks!

      - tye        

Re: getVars bug kills full-page chat
by ysth (Canon) on Dec 24, 2004 at 05:57 UTC
    Sigh. Why did this not turn up until now?

      borg's belly clears out any outdated entries. I'm guessing that there was a really old entry (probably for a certain user who would never take the hint) and today was the first time someone viewed borg's belly since that entry expired.

      Or, if you prefer, someone didn't bother to test that the old and new code returned compatible values for the edge cases such as empty vars. (:

      - tye        

        Works correctly for truly empty vars (vars=""), but not for cleared ones (vars="==01:H"). Will see what I can do.