Bug 154922 – Modern IDB: Close UniqueIDBDatabases once they become unused

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brady Eidson]

Modern IDB: Close UniqueIDBDatabases once they become unused

Besides just being a good idea, this was actually causing problems in testing on OS X as once a DatabaseProcess (or DumpRenderTree) accumulated ~256 open UniqueIDBDatabases it run up against the launchd enforced "256 open file handles" limit.

[Brady Eidson]

Created attachment 272674 [details]
Patch v1

[WebKit Commit Bot]

Attachment 272674 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:601:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Created attachment 272675 [details]
Patch v1 with more style

[Alex Christensen]

Comment on attachment 272675 [details]
Patch v1 with more style

View in context: https://bugs.webkit.org/attachment.cgi?id=272675&action=review

Please add a test that opens too many databases and hits the expected error.

> Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp:1137
> +    // Assuming it is not ephemeral, the server should now close it to free up resources.

Why do we only close non-ephemeral databases?  Shouldn't a memory backing store just not do anything when it closes?  I think this is unnecessary complexity.

[Brady Eidson]

(In reply to comment #4)
> Comment on attachment 272675 [details]
> Patch v1 with more style
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=272675&action=review
> 
> Please add a test that opens too many databases and hits the expected error.
> 
> > Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp:1137
> > +    // Assuming it is not ephemeral, the server should now close it to free up resources.
> 
> Why do we only close non-ephemeral databases?  Shouldn't a memory backing
> store just not do anything when it closes?  I think this is unnecessary
> complexity.

As discussed in person, opening, then closing, then reopening a database in private browsing needs to work - you need to get back to the same in-memory data.

The only reason we can close *persistent* databases is because we can get them back from disk.

[Brady Eidson]

Created attachment 272689 [details]
Patch for landing

[Brady Eidson]

(Alex said r+ in person, didn't twiddle the review bit here in the bug)

[Brady Eidson]

https://bugs.webkit.org/show_bug.cgi?id=154706 was caused by this bug.

[WebKit Commit Bot]

Comment on attachment 272689 [details]
Patch for landing

Clearing flags on attachment: 272689

Committed r197474: <http://trac.webkit.org/changeset/197474>