This Mediabot v3 pass was a focused reliability cleanup in Mediabot/DBCommands.pm.
The goal was not to add features.
The goal was to remove a quiet class of long-running bot problems:
DBI statement handles not closed on error paths
prepare()/execute() paths that could crash or leak cursors
admin/debug commands that could fail badly when MariaDB is unhealthy
No schema change.
Most of these bugs do not show up when the database is healthy.
They show up when something goes wrong:
MariaDB temporarily unavailable
database handle stale or broken
table missing during migration/debug
SQL prepare succeeds but execute fails
SQL prepare fails and returns undef
In those cases, old code could do things like:
$sth->execute(...)
$sth->finish
without always proving that $sth existed.
Or it could correctly guard the execute call:
unless ($sth && $sth->execute(...)) {
return;
}
but forget to close $sth when prepare() succeeded and execute() failed.
That second case is subtle: it does not always crash, but it can leak DBI cursors over time.
For a long-running IRC bot, that is exactly the kind of boring bug worth killing.
Mediabot/DBCommands.pmThis pass focused on DBCommands.pm, especially public-command database helpers and admin commands.
The audited areas included:
timers
responders
SQL-backed public commands
modcmd
chowncmd
showcmd
mvcmd
countcmd
topcmd
lastcmd
searchcmd
owncmd
holdcmd
addcatcmd
ignores
lastcom
yomomma
The responder path is important because it can run from normal public channel traffic.
Several issues were fixed around:
checkResponder()
doResponder()
checkResponder()The successful match path now closes the cursor before returning immediately.
The fallback path also avoids calling finish on an undefined statement handle:
$sth->finish if $sth;
doResponder()Both the responder lookup and the hit counter update now guard prepare() failure before execute().
The hit update now also closes its statement handle cleanly.
This matters because responders can be hit frequently on active channels.
mbDbCommand() now guards both:
SELECT command/action
UPDATE hits
This prevents a normal SQL-backed public command from crashing the dispatcher when the database is unstable.
The command can now fail cleanly instead of trying to call execute() on an undefined statement handle.
Several command-management helpers were cleaned up:
modcmd
chowncmd
showcmd
mvcmd
The recurring fix was:
unless ($sth && $sth->execute(...)) {
...
$sth->finish if $sth;
return;
}
This was applied where needed so failed execute() paths close the cursor before returning.
modcmdBoth legacy and context-based paths were checked.
The update statement now guards failed prepare() and closes handles on error.
chowncmdThe command lookup, target user lookup, and owner update now close statement handles on execute failure.
showcmdThe command display query now follows the same safe error path.
mvcmdThe rename flow was hardened:
check that new command name does not already exist
load old command
update command name
All three DB operations now close statement handles correctly on error.
The following database-backed diagnostic/admin commands were cleaned up:
countcmd
topcmd
lastcmd
searchcmd
owncmd
holdcmd
These commands are often used exactly when something feels wrong, so they should not make a database problem worse.
They now fail with controlled logging/notices and close statement handles on error.
The following areas were also cleaned up:
addcatcmd
addResponder_ctx
delResponder_ctx
IgnoresList_ctx
addIgnore_ctx
delIgnore_ctx
These are not high-volume paths, but they are long-lived administration paths.
The same DBI rule now applies consistently:
if prepare succeeded and execute failed, close the handle before return
lastcom and yomomma cleaned uplastcomThe ACTIONS_LOG query now closes its statement handle if execute fails.
yomommaAll three query paths were hardened:
specific joke by ID
COUNT(*) for random mode
random OFFSET lookup
Each branch now closes the statement handle if execute fails.
After all patches, a global scan was run against DBCommands.pm.
The most important result:
===== Scan 2: guarded execute blocks with return but no finish in block =====
was empty.
That means no remaining block was found in the risky form:
unless ($sth && $sth->execute(...)) {
...
return;
}
without a corresponding:
$sth->finish if $sth;
before returning.
The remaining direct ->execute() calls reported by the scan were manually inspected.
They all use the safe split pattern:
unless ($sth) {
...
return;
}
unless ($sth->execute(...)) {
...
$sth->finish;
return;
}
So $sth is guaranteed to exist before calling execute().
Confirmed safe areas included:
onStartTimers()
mbAddTimer_ctx()
mbRemTimer_ctx()
mbTimers_ctx()
getCommandCategory()
mbPopCommand_ctx()
Minimum validation:
perl -I. -c Mediabot/DBCommands.pm
perl -c mediabot.pl
Recommended wider validation:
find Mediabot -name '*.pm' -print -exec perl -I. -c {} \;
perl -c mediabot.pl
This pass leaves DBCommands.pm much cleaner for DBI error handling:
no known undef->execute path in the audited file
no known guarded execute + early return without finish
statement handles are closed on execute failure
responders and SQL-backed commands are safer under DB trouble
admin diagnostic commands fail more cleanly
long-running cursor leak risk is reduced
No schema change.
Scourgify DBI!
No new magic features.
Just fewer dirty cursors, fewer fragile DB paths, and a bot that should survive MariaDB hiccups with a little more dignity.
Mischief cleaned. 🧹🧙♂️
You must be logged in to reply.