Fix memory leak in file analyzer.

This undoes the changes applied in merge 9db27a6d60
and goes back to the state in the branch as of the merge 5ab3b86.

Getting rid of the additional layer of removing analyzers and just
keeping them in the set introduced subtle differences in behavior since
a few calls were still passed along. Skipping all of these with SetSkip
introduced yet other subtle behavioral differences.
This commit is contained in:
Johanna Amann 2017-02-04 16:47:07 -08:00
parent 7beac6e404
commit 1de6cfc2e3
3 changed files with 17 additions and 2 deletions

View file

@ -129,11 +129,12 @@ bool AnalyzerSet::Remove(file_analysis::Tag tag, HashKey* key)
file->GetID().c_str(), file->GetID().c_str(),
file_mgr->GetComponentName(tag).c_str()); file_mgr->GetComponentName(tag).c_str());
a->Done();
// We don't delete the analyzer object right here because the remove // We don't delete the analyzer object right here because the remove
// operation may execute at a time when it can still be accessed. // operation may execute at a time when it can still be accessed.
// Instead we let disable it; it will be deleted together with the AnalyzerSet. // Instead we let the file know to delete the analyzer later.
a->SetSkip(true); file->DoneWithAnalyzer(a);
return true; return true;
} }

View file

@ -107,6 +107,9 @@ File::~File()
DBG_LOG(DBG_FILE_ANALYSIS, "[%s] Destroying File object", id.c_str()); DBG_LOG(DBG_FILE_ANALYSIS, "[%s] Destroying File object", id.c_str());
Unref(val); Unref(val);
delete file_reassembler; delete file_reassembler;
for ( auto a : done_analyzers )
delete a;
} }
void File::UpdateLastActivityTime() void File::UpdateLastActivityTime()
@ -494,6 +497,11 @@ void File::DeliverChunk(const u_char* data, uint64 len, uint64 offset)
EndOfFile(); EndOfFile();
} }
void File::DoneWithAnalyzer(Analyzer* analyzer)
{
done_analyzers.push_back(analyzer);
}
void File::DataIn(const u_char* data, uint64 len, uint64 offset) void File::DataIn(const u_char* data, uint64 len, uint64 offset)
{ {
analyzers.DrainModifications(); analyzers.DrainModifications();

View file

@ -119,6 +119,11 @@ public:
*/ */
bool RemoveAnalyzer(file_analysis::Tag tag, RecordVal* args); bool RemoveAnalyzer(file_analysis::Tag tag, RecordVal* args);
/**
* Signal that this analyzer can be deleted once it's safe to do so.
*/
void DoneWithAnalyzer(Analyzer* analyzer);
/** /**
* Pass in non-sequential data and deliver to attached analyzers. * Pass in non-sequential data and deliver to attached analyzers.
* @param data pointer to start of a chunk of file data. * @param data pointer to start of a chunk of file data.
@ -287,6 +292,7 @@ protected:
bool postpone_timeout; /**< Whether postponing timeout is requested. */ bool postpone_timeout; /**< Whether postponing timeout is requested. */
bool done; /**< If this object is about to be deleted. */ bool done; /**< If this object is about to be deleted. */
AnalyzerSet analyzers; /**< A set of attached file analyzers. */ AnalyzerSet analyzers; /**< A set of attached file analyzers. */
std::list<Analyzer *> done_analyzers; /**< Analyzers we're done with, remembered here until they can be safely deleted. */
struct BOF_Buffer { struct BOF_Buffer {
BOF_Buffer() : full(false), size(0) {} BOF_Buffer() : full(false), size(0) {}