If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Find all async DB connections that are not asyncClose()d and cause leaks

NEW
Unassigned

Status

()

Core
General
6 years ago
2 years ago

People

(Reporter: njn, Unassigned)

Tracking

({mlk})

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

6 years ago
It's a bug if an async DB connection isn't asyncClose()d before its destroyed.  This used to cause crashes in about:memory and telemetry, but thanks to bug 662989 it now only causes leaks (which show up in about:memory).

Bug 654573 comment 60 analyzed JS code and found some places that were failing to do this, resulting in bug 662985 and 662986.  But they're probably not causing the crashes we saw before bug 662989 landed.

So there are probably some cases in C++ code where we aren't calling asyncClose().  In this bug we should hunt them down and fix them.  A code audit is probably the best approach.

Alternatively... we can tell in Connection's destructor if it hasn't been asyncClosed()d.  Maybe it's possible to force the connection to close in a safe way, thus rendering this error-prone API safer?  We'd just have to add an event to the main loop that frees the Connection's mDBConn.

Updated

6 years ago
Crash Signature: [@ mozilla::storage::StorageMemoryReporter::GetPath ] [@ @0x0 | mozilla::storage::StorageMemoryReporter::GetPath ] [@ mozilla::storage::Connection::getFilename() ] [@ mozilla::storage::Connection::getFilename ]
(Reporter)

Comment 1

6 years ago
Scoobidiver: you added a crash signature, but with bug 662989 landed those crashes shouldn't happen any more.  Have you seen that crash signature since bug 662989 landed, or did you just copy it over from that bug?

Comment 2

6 years ago
(In reply to Nicholas Nethercote [:njn] from comment #1)
> Have you seen that crash signature since bug 662989 landed
No:
https://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A8.0a1&signature=mozilla%3A%3Astorage%3A%3AConnection%3A%3AgetFilename%28%29
https://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A8.0a1&signature=mozilla%3A%3Astorage%3A%3AStorageMemoryReporter%3A%3AGetPath
https://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A8.0a1&signature=%400x0%20|%20mozilla%3A%3Astorage%3A%3AStorageMemoryReporter%3A%3AGetPath

bug 662989 fixes some causes, but I wasn't sure it fixes all causes that is why I added these crash signatures to this follow-up bug, just in case.
Feel free to remove them if the remaining things are only about memory leaks.
Crash Signature: [@ mozilla::storage::StorageMemoryReporter::GetPath ] [@ @0x0 | mozilla::storage::StorageMemoryReporter::GetPath ] [@ mozilla::storage::Connection::getFilename() ] [@ mozilla::storage::Connection::getFilename ]
Keywords: mlk
Summary: Find all async DB connections that are not asyncClose()d → Find all async DB connections that are not asyncClose()d and cause leaks
You need to log in before you can comment on or make changes to this bug.