Limit the number of crash reports to enqueue at once
BUG=5357
Change-Id: Ib21086cdd34c938def885d625a165ba2fa0879a4
Review URL: http://codereview.chromium.org/3209003
diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc
index 492f2e8..0194fc4 100644
--- a/crash_reporter/crash_collector.cc
+++ b/crash_reporter/crash_collector.cc
@@ -4,6 +4,7 @@
#include "crash-reporter/crash_collector.h"
+#include <dirent.h>
#include <pwd.h> // For struct passwd.
#include <sys/types.h> // for mode_t.
@@ -25,6 +26,9 @@
static const uid_t kRootOwner = 0;
static const uid_t kRootGroup = 0;
+// Maximum of 8 crash reports per directory.
+const int CrashCollector::kMaxCrashDirectorySize = 8;
+
CrashCollector::CrashCollector() : forced_crash_directory_(NULL) {
}
@@ -146,5 +150,46 @@
return false;
}
+ if (!CheckHasCapacity(*crash_directory)) {
+ return false;
+ }
+
return true;
}
+
+// Return true if the given crash directory has not already reached
+// maximum capacity.
+bool CrashCollector::CheckHasCapacity(const FilePath &crash_directory) {
+ DIR* dir = opendir(crash_directory.value().c_str());
+ if (!dir) {
+ return false;
+ }
+ struct dirent ent_buf;
+ struct dirent* ent;
+ int count_non_core = 0;
+ int count_core = 0;
+ bool full = false;
+ while (readdir_r(dir, &ent_buf, &ent) == 0 && ent != NULL) {
+ if ((strcmp(ent->d_name, ".") == 0) ||
+ (strcmp(ent->d_name, "..") == 0))
+ continue;
+
+ if (strcmp(ent->d_name + strlen(ent->d_name) - 5, ".core") == 0) {
+ ++count_core;
+ } else {
+ ++count_non_core;
+ }
+
+ if (count_core >= kMaxCrashDirectorySize ||
+ count_non_core >= kMaxCrashDirectorySize) {
+ logger_->LogWarning(
+ "Crash directory %s already full with %d pending reports",
+ crash_directory.value().c_str(),
+ kMaxCrashDirectorySize);
+ full = true;
+ break;
+ }
+ }
+ closedir(dir);
+ return !full;
+}
diff --git a/crash_reporter/crash_collector.h b/crash_reporter/crash_collector.h
index ef0d2a7..a75d791 100644
--- a/crash_reporter/crash_collector.h
+++ b/crash_reporter/crash_collector.h
@@ -32,10 +32,15 @@
protected:
friend class CrashCollectorTest;
+ FRIEND_TEST(CrashCollectorTest, CheckHasCapacityOverCore);
+ FRIEND_TEST(CrashCollectorTest, CheckHasCapacityOverNonCore);
FRIEND_TEST(CrashCollectorTest, GetCrashDirectoryInfo);
FRIEND_TEST(CrashCollectorTest, FormatDumpBasename);
FRIEND_TEST(CrashCollectorTest, Initialize);
+ // Set maximum enqueued crashes in a crash directory.
+ static const int kMaxCrashDirectorySize;
+
// For testing, set the directory always returned by
// GetCreatedCrashDirectoryByEuid.
void ForceCrashDirectory(const char *forced_directory) {
@@ -54,13 +59,19 @@
// Determines the crash directory for given eud, and creates the
// directory if necessary with appropriate permissions. Returns
// true whether or not directory needed to be created, false on any
- // failure.
+ // failure. If the crash directory is already full, returns false.
bool GetCreatedCrashDirectoryByEuid(uid_t euid,
FilePath *crash_file_path);
+
+ // Format crash name based on components.
std::string FormatDumpBasename(const std::string &exec_name,
time_t timestamp,
pid_t pid);
+ // Check given crash directory still has remaining capacity for another
+ // crash.
+ bool CheckHasCapacity(const FilePath &crash_directory);
+
CountCrashFunction count_crash_function_;
IsFeedbackAllowedFunction is_feedback_allowed_function_;
SystemLogging *logger_;
diff --git a/crash_reporter/crash_collector_test.cc b/crash_reporter/crash_collector_test.cc
index 48b7fa0..ed8fff7 100644
--- a/crash_reporter/crash_collector_test.cc
+++ b/crash_reporter/crash_collector_test.cc
@@ -5,6 +5,7 @@
#include <unistd.h>
#include "base/file_util.h"
+#include "base/string_util.h"
#include "crash-reporter/crash_collector.h"
#include "crash-reporter/system_logging_mock.h"
#include "gflags/gflags.h"
@@ -20,15 +21,25 @@
}
class CrashCollectorTest : public ::testing::Test {
+ public:
void SetUp() {
collector_.Initialize(CountCrash,
IsMetrics,
&logging_);
+ test_dir_ = FilePath("test");
+ file_util::CreateDirectory(test_dir_);
}
+
+ void TearDown() {
+ file_util::Delete(test_dir_, true);
+ }
+
+ bool CheckHasCapacity();
+
protected:
SystemLoggingMock logging_;
CrashCollector collector_;
- pid_t pid_;
+ FilePath test_dir_;
};
TEST_F(CrashCollectorTest, Initialize) {
@@ -99,6 +110,49 @@
ASSERT_EQ("foo.20100523.135015.100", basename);
}
+bool CrashCollectorTest::CheckHasCapacity() {
+ static const char kFullMessage[] = "Crash directory test already full";
+ bool has_capacity = collector_.CheckHasCapacity(test_dir_);
+ bool has_message = (logging_.log().find(kFullMessage) != std::string::npos);
+ EXPECT_EQ(has_message, !has_capacity);
+ return has_capacity;
+}
+
+TEST_F(CrashCollectorTest, CheckHasCapacityOverNonCore) {
+ // Test up to kMaxCrashDirectorySize-1 non-core files can be added.
+ for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize - 1; ++i) {
+ EXPECT_TRUE(CheckHasCapacity());
+ file_util::WriteFile(test_dir_.Append(StringPrintf("file%d", i)), "", 0);
+ }
+
+ // Test an additional kMaxCrashDirectorySize - 1 core files fit.
+ for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize - 1; ++i) {
+ EXPECT_TRUE(CheckHasCapacity());
+ file_util::WriteFile(test_dir_.Append(StringPrintf("file%d.core", i)),
+ "", 0);
+ }
+
+ // Test an additional kMaxCrashDirectorySize non-core files don't fit.
+ for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize; ++i) {
+ file_util::WriteFile(test_dir_.Append(StringPrintf("overage%d", i)), "", 0);
+ EXPECT_FALSE(CheckHasCapacity());
+ }
+}
+
+TEST_F(CrashCollectorTest, CheckHasCapacityOverCore) {
+ // Set up kMaxCrashDirectorySize - 1 core files.
+ for (int i = 0; i < CrashCollector::kMaxCrashDirectorySize - 1; ++i) {
+ file_util::WriteFile(test_dir_.Append(StringPrintf("file%d.core", i)),
+ "", 0);
+ }
+
+ EXPECT_TRUE(CheckHasCapacity());
+
+ // Test an additional core file does not fit.
+ file_util::WriteFile(test_dir_.Append("overage.core"), "", 0);
+ EXPECT_FALSE(CheckHasCapacity());
+}
+
int main(int argc, char **argv) {
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
