Skip to content

Commit

Permalink
Unprotected code loaning samples (#2086)
Browse files Browse the repository at this point in the history
* Refs #11831. Fix unprotected code accessing writer pool

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #11831. Fix wrong test testing dirty payloads

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
  • Loading branch information
richiware authored Jul 22, 2021
1 parent 439d937 commit e77cc9a
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 14 deletions.
4 changes: 4 additions & 0 deletions src/cpp/fastdds/publisher/DataWriterImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,8 @@ ReturnCode_t DataWriterImpl::loan_sample(
return ReturnCode_t::RETCODE_NOT_ENABLED;
}

std::lock_guard<RecursiveTimedMutex> lock(writer_->getMutex());

// Get one payload from the pool
PayloadInfo_t payload;
uint32_t size = type_->m_typeSize;
Expand Down Expand Up @@ -466,6 +468,8 @@ ReturnCode_t DataWriterImpl::discard_loan(
return ReturnCode_t::RETCODE_NOT_ENABLED;
}

std::lock_guard<RecursiveTimedMutex> lock(writer_->getMutex());

// Remove sample from loans collection
PayloadInfo_t payload;
if ((nullptr == sample) || !check_and_remove_loan(sample, payload))
Expand Down
19 changes: 5 additions & 14 deletions test/blackbox/common/DDSBlackboxTestsDataSharing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,23 +201,14 @@ TEST(DDSDataSharing, ReliableDirtyPayloads)
std::advance(data_it, writer_sent_data - writer_history_depth - 1);
std::copy(data_it, data.end(), std::back_inserter(valid_data));

writer.send(data, 100);
ASSERT_TRUE(data.empty());

// Send the data to fill the history and overwrite old changes
// The reader will receive and process all changes so that the writer can reuse them,
// but will keep them in the history.
read_reader.startReception(data);
writer.send(data, 100);
ASSERT_TRUE(data.empty());
read_reader.startReception(valid_data);
read_reader.block_for_all();

// Doing a second read on the same history, the application will see only the last samples
while (!valid_data.empty())
{
FixedSized value;
ASSERT_TRUE(read_reader.take_first_data(&value));
ASSERT_EQ(valid_data.front(), value);
valid_data.pop_front();
}
ASSERT_TRUE(valid_data.empty());
}

TEST(DDSDataSharing, DataSharingWriter_DifferentDomainReaders)
Expand Down Expand Up @@ -497,4 +488,4 @@ TEST(DDSDataSharing, DataSharingPoolError)
writer_auto.send(data);
ASSERT_TRUE(data.empty());
reader.block_for_all();
}
}

0 comments on commit e77cc9a

Please sign in to comment.