Chromium Code Reviews Chromium Code Reviews
Issue
16510002:
Better ActivityLog error handling (Closed)
DescriptionWe were seeing ActivityLog memory leaks and assorted other errors. It was due to poor handling of error conditions (not cleaning up the DB state if something goes wrong). This CL should fix that.
Added every error-handling measure I could think of:
1. Moves the error handling into the ActivityDatabase class so that we can immediately (synchronously) kill the database, instead of asynchronously doing it from the ActivityLog
2. Closes the db even for non-catastrophic errors, so that we aren't just hammering away when the I/O thread has gone wonky
3. Adds checks in ActivityDatabase to see if the db is valid so that we aren't constantly continuing to try to write to a closed database
4. Removes dead code that I had superstitiously added to the activity_log_unittest to see if adding it would remove the memory leaks (it didn't)
5. Failures in Init explicitly call the error handling code, in case it isn't automatically triggered
6. Record now returns a bool, so we can stop trying to record actions if one has failed
7. In the case of unittests where everything is running on one thread anyway, ActivityLog synchronously kills the Activity Database
8. ActivityLog does cleanup on Shutdown, which happens before references start dying
BUG=246825
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=205059
Patch Set 1 #Patch Set 2 : Forgot to remove suppressions-- these trybot runs will be more meaningful #Patch Set 3 : Added more error precautions #Patch Set 4 : Trying to fix error callback bug #Patch Set 5 : git cl try #Patch Set 6 : Adding more error checking #Patch Set 7 : Reinstated lines commented out for testing #Patch Set 8 : Removed unnecessary call to close #Patch Set 9 : Cleanup before review #
Total comments: 13
Patch Set 10 : Fixed nits #Patch Set 11 : Reinstating the command line #Messages
Total messages: 10 (0 generated)
|
