Skip to content

Commit

Permalink
[Recorder]: Acquire lock for ofstream changes (#1145)
Browse files Browse the repository at this point in the history
While the Recorder is writing to the sairedis log file, it's possible for a log rotate to occur at exactly the right time so that file stream used for writing (a std::ofstream) is re-opened in the middle of the write operation (since writing and log rotate are handled by separate threads). Since standard library objects are not thread safe, this can cause some pointers used during the write operation to be overwritten, leading to a segmentation fault when the write operation proceeds.

To prevent this from occurring, acquire a lock for the file stream for any methods that change the ofstream (including opening, closing, and writing to it with the << operator). Also use recordLine for all writes to the file stream to avoid deadlock.

Signed-off-by: Lawrence Lee lawlee@microsoft.com
  • Loading branch information
theasianpianist authored and yxieca committed Nov 29, 2022
1 parent 57fcb47 commit 98def2d
Showing 1 changed file with 13 additions and 9 deletions.
22 changes: 13 additions & 9 deletions lib/Recorder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,13 @@ void Recorder::requestLogRotate()

/* double check since reopen could fail */

if (m_ofstream.is_open())
{
m_ofstream << getTimestamp() << "|" << "#|logrotate on: " << m_recordingFile << std::endl;
}
recordLine("#|logrotate on: " + m_recordingFile);
}

void Recorder::recordingFileReopen()
{
MUTEX();

SWSS_LOG_ENTER();

m_ofstream.close();
Expand Down Expand Up @@ -224,12 +223,15 @@ void Recorder::startRecording()

m_recordingFile = m_recordingOutputDirectory + "/" + m_recordingFileName;

m_ofstream.open(m_recordingFile, std::ofstream::out | std::ofstream::app);

if (!m_ofstream.is_open())
{
SWSS_LOG_ERROR("failed to open recording file %s: %s", m_recordingFile.c_str(), strerror(errno));
return;
MUTEX();
m_ofstream.open(m_recordingFile, std::ofstream::out | std::ofstream::app);

if (!m_ofstream.is_open())
{
SWSS_LOG_ERROR("failed to open recording file %s: %s", m_recordingFile.c_str(), strerror(errno));
return;
}
}

recordLine("#|recording on: " + m_recordingFile);
Expand All @@ -239,6 +241,8 @@ void Recorder::startRecording()

void Recorder::stopRecording()
{
MUTEX();

SWSS_LOG_ENTER();

SWSS_LOG_NOTICE("stopped recording");
Expand Down

0 comments on commit 98def2d

Please sign in to comment.