Bug #5622
closedCaptive Portal database handling can reset the DB while it is locked when it should wait instead
0%
Description
From a user-submitted message:
[snip]
etc/inc/captiveportal.inc:captiveportal_opendb() will reinitialize the
database(-file) if the file exists, but an error happens during query
execution. Because this can be a simple database lock (e.g. by minicron
job to prune old entries, or by a login happening at the same time),
chances are relatively that this happens sooner or later.The relevant loc is
https://github.com/pfsense/pfsense/blob/master/src/etc/inc/captiveportal.inc#L1440
[snip]
Possible "solutions":
1) give opendb a "retry" like it already does e.g. in
https://github.com/pfsense/pfsense/blob/master/src/etc/inc/captiveportal.inc#
if (! $DB->exec($createquery)) { captiveportal_syslog("Error during table {$cpzone} creation. Error message: {$DB->lastErrorMsg()}. Trying again."); # sleep (0.1) // Did I mention I dont know php much? if (! $DB->exec ... captiveportal_syslog("Still error during table {$cpzone} creation.... Resetting");
2) Explicitely check whether we ran into a lock and retry. This should
be done using $DB->lastErrorCode()
a) do retry x times/seconds or
b) wait to get the lock forever.Both approaches will not forever solve the issue, but drastically
lowering the chances that we have "pass-throughs" without being able to
easily spot them via the CP pages.
If the DB is reset while users are active, IPFW rules are left in place which allow some users to inadvertently retain access longer than they should. If the DB is reset the IPFW tables should be reset as well, but it looks like it should also wait a brief period for the lock to clear before resetting if it's locked and not actually corrupt.
Updated by Chris Buechler about 9 years ago
- Status changed from New to Confirmed
re: cleaning up ipfw rules after the database is reinitialized, the reinit=true doesn't exist as it used to in 2.0x.
https://github.com/pfsense/pfsense/blob/RELENG_2_0/etc/inc/captiveportal.inc#L586
That was removed as part of the graceful reloading of CP, but needs to come back for the database re-initialization scenario to ensure nothing's left behind in the tables.
Updated by Jose Carlos Bilhega about 9 years ago
I running about 2 days without any erros ( crashes or locking logs ) with just the use of busytimeout added on the line 1430:
...
$DB->busyTimeout(5000);
if (! $DB->exec($createquery)) {
...
reference: http://php.net/manual/en/sqlite3.busytimeout.php
I hope this is a good solution.
Updated by Chris Buechler about 9 years ago
- Status changed from Confirmed to Feedback
- Assignee set to Chris Buechler
believe this to be fixed, leaving for additional feedback.