Project

General

Profile

Bug #5622

Captive Portal database handling can reset the DB while it is locked when it should wait instead

Added by Jim Pingle almost 3 years ago. Updated almost 3 years ago.

Status:
Resolved
Priority:
High
Category:
Captive Portal
Target version:
Start date:
12/10/2015
Due date:
% Done:

0%

Estimated time:
Affected Version:
2.2.5
Affected Architecture:
All

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.

Associated revisions

Revision 7c4b3b20 (diff)
Added by Chris Buechler almost 3 years ago

Add busytimeout of 60 seconds for CP database access. This matches the default value PHP uses with sqlite 2.x versions (pfSense 2.1.x and earlier) and prevents the issues noted in Ticket #5622

Revision 18ca572b (diff)
Added by Chris Buechler almost 3 years ago

Add busytimeout of 60 seconds for CP database access. This matches the default value PHP uses with sqlite 2.x versions (pfSense 2.1.x and earlier) and prevents the issues noted in Ticket #5622

Revision 07917f7d (diff)
Added by Chris Buechler almost 3 years ago

Flush zone's tables if its db must be reset to avoid leaving behind any table entries. Ticket #5622

Revision 5e71234e (diff)
Added by Chris Buechler almost 3 years ago

Flush zone's tables if its db must be reset to avoid leaving behind any table entries. Ticket #5622

History

#1 Updated by Chris Buechler almost 3 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.

#2 Updated by Jose Carlos Bilhega almost 3 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.

#3 Updated by Chris Buechler almost 3 years ago

  • Status changed from Confirmed to Feedback
  • Assignee set to Chris Buechler

believe this to be fixed, leaving for additional feedback.

#4 Updated by Chris Buechler almost 3 years ago

  • Status changed from Feedback to Resolved

fixed

Also available in: Atom PDF