Remove the BroFile cache

GH-375

src/File.cc

@@ -28,16 +28,10 @@
 #include "Event.h"
 #include "Reporter.h"
 
-// The following could in principle be part of a "file manager" object.
+std::list<std::pair<std::string, BroFile*>> BroFile::open_files;
 
-#define MAX_FILE_CACHE_SIZE 512
-static int num_files_in_cache = 0;
-static BroFile* head = 0;
-static BroFile* tail = 0;
-
-// Maximizes the number of open file descriptors and returns the number
-// that we should use for the cache.
-static int maximize_num_fds()
+// Maximizes the number of open file descriptors.
+static void maximize_num_fds()
 	{
 	struct rlimit rl;
 	if ( getrlimit(RLIMIT_NOFILE, &rl) < 0 )
@@ -46,11 +40,7 @@ static int maximize_num_fds()
 	if ( rl.rlim_max == RLIM_INFINITY )
 		{
 		// Don't try raising the current limit.
-		if ( rl.rlim_cur == RLIM_INFINITY )
-			// Let's not be too ambitious.
-			return MAX_FILE_CACHE_SIZE;
-		else
-			return rl.rlim_cur / 2;
+		return;
 		}
 
 	// See if we can raise the current to the maximum.
@@ -58,11 +48,8 @@ static int maximize_num_fds()
 
 	if ( setrlimit(RLIMIT_NOFILE, &rl) < 0 )
 		reporter->FatalError("maximize_num_fds(): setrlimit failed");
-
-	return rl.rlim_cur / 2;
 	}
 
-
 BroFile::BroFile(FILE* arg_f)
 	{
 	Init();
@@ -104,7 +91,6 @@ BroFile::BroFile(const char* arg_name, const char* arg_access, BroType* arg_t)
 		{
 		reporter->Error("cannot open %s: %s", name, strerror(errno));
 		is_open = 0;
-		okay_to_manage = 0;
 		}
 	}
 
@@ -127,14 +113,15 @@ const char* BroFile::Name() const
 
 bool BroFile::Open(FILE* file, const char* mode)
 	{
+	static bool fds_maximized = false;
 	open_time = network_time ? network_time : current_time();
 
-	if ( ! max_files_in_cache )
+	if ( ! fds_maximized )
+		{
 		// Haven't initialized yet.
-		max_files_in_cache = maximize_num_fds();
-
-	if ( num_files_in_cache >= max_files_in_cache )
-		PurgeCache();
+		maximize_num_fds();
+		fds_maximized = true;
+		}
 
 	f = file;
 
@@ -148,22 +135,15 @@ bool BroFile::Open(FILE* file, const char* mode)
 
 	SetBuf(buffered);
 
-	if ( f )
+	if ( ! f )
 		{
-		// These are the only files we manage, because we open them
-		// ourselves and hence don't have any surprises regarding
-		// whether we're allowed to close them.
-		is_open = okay_to_manage = 1;
-
-		InsertAtBeginning();
-		}
-	else
-		{
-		// No point managing it.
-		is_open = okay_to_manage = 0;
+		is_open = 0;
 		return false;
 		}
 
+	is_open = 1;
+	open_files.emplace_back(std::make_pair(name, this));
+
 	RaiseOpenEvent();
 
 	return true;
@@ -185,9 +165,7 @@ BroFile::~BroFile()
 
 void BroFile::Init()
 	{
-	is_open = okay_to_manage = is_in_cache = 0;
-	position = 0;
-	next = prev = 0;
+	is_open = 0;
 	attrs = 0;
 	buffered = true;
 	print_hook = true;
@@ -201,56 +179,6 @@ void BroFile::Init()
 
 FILE* BroFile::File()
 	{
-	if ( okay_to_manage && ! is_in_cache )
-		f = BringIntoCache();
-
-	return f;
-	}
-
-FILE* BroFile::BringIntoCache()
-	{
-	char buf[256];
-
-	if ( f )
-		reporter->InternalError("BroFile non-nil non-open file");
-
-	if ( num_files_in_cache >= max_files_in_cache )
-		PurgeCache();
-
-	if ( position == 0 )
-		// Need to truncate it.
-		f = fopen(name, access);
-	else
-		// Don't clobber it.
-		f = fopen(name, "a");
-
-	if ( ! f )
-		{
-		bro_strerror_r(errno, buf, sizeof(buf));
-		reporter->Error("can't open %s: %s", name, buf);
-
-		f = fopen("/dev/null", "w");
-
-		if ( f )
-			{
-			okay_to_manage = 0;
-			return f;
-			}
-
-		bro_strerror_r(errno, buf, sizeof(buf));
-		reporter->Error("can't open /dev/null: %s", buf);
-		return 0;
-		}
-
-	if ( fseek(f, position, SEEK_SET) < 0 )
-		{
-		bro_strerror_r(errno, buf, sizeof(buf));
-		reporter->Error("reopen seek failed: %s", buf);
-		}
-
-	InsertAtBeginning();
-	RaiseOpenEvent();
-
 	return f;
 	}
 
@@ -285,126 +213,30 @@ int BroFile::Close()
 	if ( f == stdin || f == stdout || f == stderr )
 		return 0;
 
-	if ( is_in_cache )
-		{
-		Unlink();
-		if ( f )
-			{
-			fclose(f);
-			f = 0;
-			open_time = 0;
-			}
-
-		is_open = 0;
-		okay_to_manage = 0; // no longer managed since will never reopen
-
-		return 1;
-		}
-
-	// Not managed.
 	if ( ! f )
 		return 0;
 
 	fclose(f);
-	f = 0;
+	f = nullptr;
+	open_time = is_open = 0;
+
+	Unlink();
 
 	return 1;
 	}
 
-void BroFile::Suspend()
-	{
-	if ( ! is_in_cache )
-		reporter->InternalError("BroFile::Suspend() called for non-cached file");
-
-	if ( ! is_open )
-		reporter->InternalError("BroFile::Suspend() called for non-open file");
-
-	Unlink();
-
-	if ( ! f )
-		reporter->InternalError("BroFile::Suspend() called for nil file");
-
-	if ( (position = ftell(f)) < 0 )
-		{
-		char buf[256];
-		bro_strerror_r(errno, buf, sizeof(buf));
-		reporter->Error("ftell failed: %s", buf);
-		position = 0;
-		}
-
-	fclose(f);
-	f = 0;
-	}
-
-void BroFile::PurgeCache()
-	{
-	if ( tail )
-		{
-		tail->Suspend();
-		return;
-		}
-
-	reporter->InternalWarning("BroFile purge of empty cache");
-	}
-
 void BroFile::Unlink()
 	{
-	if ( is_in_cache )
+	for ( auto it = open_files.begin(); it != open_files.end(); ++it)
 		{
-		if ( head == this )
-			head = Next();
-		else
-			Prev()->SetNext(next);
-
-		if ( tail == this )
-			tail = Prev();
-		else
-			Next()->SetPrev(prev);
-
-		if ( (head || tail) && ! (head && tail) )
-			reporter->InternalError("BroFile link list botch");
-
-		is_in_cache = 0;
-		prev = next = 0;
-
-		if ( --num_files_in_cache < 0 )
-			reporter->InternalError("BroFile underflow of file cache");
+		if ( (*it).second == this )
+			{
+			open_files.erase(it);
+			return;
+			}
 		}
 	}
 
-void BroFile::InsertAtBeginning()
-	{
-	if ( ! head )
-		{
-		head = tail = this;
-		next = prev = 0;
-		}
-	else
-		{
-		SetNext(head);
-		SetPrev(0);
-		head->SetPrev(this);
-		head = this;
-		}
-
-	if ( ++num_files_in_cache > max_files_in_cache )
-		reporter->InternalError("BroFile overflow of file cache");
-
-	is_in_cache = 1;
-	}
-
-void BroFile::MoveToBeginning()
-	{
-	if ( head == this )
-		return;	// already at the beginning
-
-	if ( ! is_in_cache || ! prev )
-		reporter->InternalError("BroFile inconsistency in MoveToBeginning()");
-
-	Unlink();
-	InsertAtBeginning();
-	}
-
 void BroFile::Describe(ODesc* d) const
 	{
 	d->AddSP("file");
@@ -444,9 +276,6 @@ RecordVal* BroFile::Rotate()
 	if ( f == stdin || f == stdout || f == stderr )
 		return 0;
 
-	if ( okay_to_manage && ! is_in_cache )
-		BringIntoCache();
-
 	RecordVal* info = new RecordVal(rotate_info);
 	FILE* newf = rotate_file(name, info);
 
@@ -459,6 +288,7 @@ RecordVal* BroFile::Rotate()
 	info->Assign(2, new Val(open_time, TYPE_TIME));
 
 	Unlink();
+
  	fclose(f);
 	f = 0;
 
@@ -466,14 +296,13 @@ RecordVal* BroFile::Rotate()
 	return info;
 	}
 
-void BroFile::CloseCachedFiles()
+void BroFile::CloseOpenFiles()
 	{
-	BroFile* next;
-	for ( BroFile* f = head; f; f = next )
+	auto it = open_files.begin();
+	while ( it != open_files.end() )
 		{
-		next = f->next;
-		if ( f->is_in_cache )
-			f->Close();
+		auto el = it++;
+		(*el).second->Close();
 		}
 	}
 
@@ -482,9 +311,6 @@ int BroFile::Write(const char* data, int len)
 	if ( ! is_open )
 		return 0;
 
-	if ( ! is_in_cache && okay_to_manage )
-		BringIntoCache();
-
 	if ( ! len )
 		len = strlen(data);
 
@@ -519,10 +345,13 @@ double BroFile::Size()
 
 BroFile* BroFile::GetFile(const char* name)
 	{
-	for ( BroFile* f = head; f; f = f->next )
+	for ( const auto &el : open_files )
 		{
-		if ( f->name && streq(name, f->name) )
-			return f;
+		if ( el.first == name )
+			{
+			Ref(el.second);
+			return el.second;
+			}
 		}
 
 	return new BroFile(name, "w", 0);