Forum teuk.org

🛡️ Protego DBI: Hardening Mediabot’s SQL Error Paths

in Mediabot · started by TeuK · 4w ago

TeuK · 4w ago

This maintenance pass focused on a quiet but important class of stability bugs in Mediabot v3: DBI statement handles that could be left open when a SQL execute() failed and the code returned early.

These bugs are not spectacular. They do not usually explode immediately. They hide in rarely triggered error paths, waiting for a database hiccup, a failed query, a missing table, or an unexpected runtime condition. Over time, they can turn into leaked cursors, confusing behavior, or avoidable instability.

So this chapter was a defensive cleanup pass.

What was fixed

The audit focused on patterns like this:

unless ($sth && $sth->execute(...)) {
    ...
    return;
}

When prepare() succeeds but execute() fails, $sth exists and should be closed before returning. The fix is simple, but applying it safely across the codebase required a careful module-by-module pass:

unless ($sth && $sth->execute(...)) {
    ...
    $sth->finish if $sth;
    return;
}

The same logic was applied to variants using handles such as $sth_c, $sth_count, $chk, $ins, $sth_up, and others.

Modules covered

This chapter touched a wide part of the bot:

  • ChannelCommands.pm
  • Helpers.pm
  • LoginCommands.pm
  • External.pm
  • Hailo.pm
  • Mediabot.pm
  • Quotes.pm
  • Partyline.pm
  • UserCommands.pm

The work included both targeted fixes and broader scans for recurring patterns.

Highlights

Partyline hardening

Partyline.pm had several database error paths where a failed query could return immediately without closing the active statement handle. These paths included authentication, karma, stats, logs, and channel-related Partyline commands.

Some returns were written inline, for example:

$stream->write("DB error.\r\n"); return;

The patching logic had to be adjusted to handle those compact forms safely.

Quotes cleanup

The quote system was hardened across add, delete, view, search, random, stats, count, and by-nick lookup paths.

The last remaining direct execute() calls were manually inspected and confirmed to close their handles correctly on failure.

UserCommands cleanup

UserCommands.pm was the biggest pass. The broad guarded-execute scan was cleaned, then remaining direct execute() calls were reviewed.

Extra hardening was applied to:

  • userStats_ctx()
  • deliverReminders()
  • KARMA_LOG persistence
  • TRIVIA_SCORES persistence

The goal was not only to stop leaks, but also to keep failure handling predictable and quiet where the feature should degrade gracefully.

Mediabot core cleanup

Mediabot.pm had three real guarded-execute return paths that needed cleanup. These are now fixed, and the remaining direct execute() calls were left for future targeted review because they were outside the main MB76 bug family.

Final validation

The final global scan was clean:

===== MB76 GLOBAL DBI SCAN — SCAN 2 ONLY =====

OK: no guarded execute return block missing finish found.

Done.

All Perl modules were also syntax-checked successfully:

find Mediabot -name '*.pm' -print -exec perl -I. -c {} \;
perl -c mediabot.pl

Every module returned syntax OK, and mediabot.pl also passed.

Result

This closes the MB76 chapter.

Mediabot now handles this class of DBI error path more safely across its major modules. It is the kind of cleanup that users may never directly notice, but the bot will: fewer leaked handles, cleaner error exits, and better long-running stability.

Not glamorous. Very necessary.

Protego DBI.

You must be logged in to reply.