Closed Bug 506027 Opened 15 years ago Closed 6 years ago

Analysis for SQLite lock usage

Categories

(Developer Infrastructure :: Source Code Analysis, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INACTIVE

People

(Reporter: sdwilsh, Unassigned)

References

Details

I'd like to be able to annotate functions to make sure that sqlite3_mutex_enter is never called during the course of the method. This would also have to make sure that methods this calls doesn't do that (I'm not sure if that's possible). This will help enforce invariants like the ones being added in bug 506022.
To do this at compile time you'd have annotate the transitive set of functions called... or we could try to start doing global analysis using a database but that gets tricky pretty quickly.
We really do need that mozilla callgraph for things like this, but in the meantime, how about restricting lock usage based on paths? Could we assume that no methods in /storage/ can call sqlite3_mutex_enter unless whitelisted?
Well, I'm more worried about calling a sqlite3 api that ends up locking that we think (or thought at some point) does not lock. We almost never actually call sqlite3_mutex_enter ourselves.
The places unit test failures aren't big - it just has to do with the fact that statements are not being finalized early enough, so sqlite3_close fails, which causes an assertion.
(ignore last comment - was meant for a different bug)
Shawn, you might try asking the SQLite people to add that information to their documentation --- for each function, an note saying whether it may lock. That'd be generally useful.
Sure, but that'd still mean that I'd have to check if that changes between their releases. Additionally, every code change would have to be verified by hand to make sure we don't accidentally call one of these functions. The benefit of having the analysis is that a machine does this checking for us.
We could do this easily if we had a callgraph. Working on that...
(In reply to comment #7) > Sure, but that'd still mean that I'd have to check if that changes between > their releases. Additionally, every code change would have to be verified by > hand to make sure we don't accidentally call one of these functions. The > benefit of having the analysis is that a machine does this checking for us. Agreed. I proposed this because we don't have the infrastructure for this analysis (yet), and based on this (In reply to comment #3) > Well, I'm more worried about calling a sqlite3 api that ends up locking that we > think (or thought at some point) does not lock. We almost never actually call > sqlite3_mutex_enter ourselves.
(In reply to comment #6) > Shawn, you might try asking the SQLite people to add that information to their > documentation --- for each function, an note saying whether it may lock. > That'd be generally useful. It would be an interesting exercise to try to push static analysis into an upstream project. Annotating functions will definitely be useful from a documentation perspective and will scale well, plus it would be a good testcase for dehydra C stuff :)
Okay, I've got a working callgraph that I could use for this. Dealing with virtual methods (nsIFoo) and figuring out which interfaces are implemented by which classes is a trickier dynamic problem, and likewise for function pointers, but if we're just talking simple calls within sqlite itself then I can do this analysis now. Find me offline, sdwilsh?
(In reply to comment #11) > Find me offline, sdwilsh? Monday - not in today.
(In reply to comment #11) > Okay, I've got a working callgraph that I could use for this. Dealing with > virtual methods (nsIFoo) and figuring out which interfaces are implemented by > which classes is a trickier dynamic problem, and likewise for function > pointers, but if we're just talking simple calls within sqlite itself then I > can do this analysis now. Find me offline, sdwilsh? Awesome! Got a bug # for that work?
Product: Core → Firefox Build System
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INACTIVE
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.