Skip to content

Commit

Permalink
Fix Leak of Instance Handles in DataReader
Browse files Browse the repository at this point in the history
Also document the `bit_autopurge_nowriter_samples_delay` and
`bit_autopurge_disposed_samples_delay` config props.
  • Loading branch information
iguessthislldo committed Aug 28, 2024
1 parent 34b0ea4 commit fd485a9
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 14 deletions.
26 changes: 14 additions & 12 deletions dds/DCPS/DataReaderImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -585,19 +585,17 @@ DataReaderImpl::remove_associations_i(const WriterIdSeq& writers,
}

for (CORBA::ULong i = 0; i < updated_writers.length(); ++i) {
{
this->disassociate(updated_writers[i]);
}
disassociate(updated_writers[i]);
}

// Mirror the add_associations SUBSCRIPTION_MATCHED_STATUS processing.
if (!this->is_bit_) {
ACE_Guard<ACE_Recursive_Thread_Mutex> justMe(publication_handle_lock_);

// Derive the change in the number of publications writing to this reader.
int matchedPublications = static_cast<int>(this->publication_id_to_handle_map_.size());
this->subscription_match_status_.current_count_change
= matchedPublications - this->subscription_match_status_.current_count;
const int matchedPublications = static_cast<int>(publication_id_to_handle_map_.size());
subscription_match_status_.current_count_change =
matchedPublications - subscription_match_status_.current_count;

// Only process status if the number of publications has changed.
if (this->subscription_match_status_.current_count_change != 0) {
Expand All @@ -606,15 +604,12 @@ DataReaderImpl::remove_associations_i(const WriterIdSeq& writers,
/// Section 7.1.4.1: total_count will not decrement.

/// @TODO: Reconcile this with the verbiage in section 7.1.4.1
this->subscription_match_status_.last_publication_handle
= handles[ wr_len - 1];
subscription_match_status_.last_publication_handle = handles[wr_len - 1];

set_status_changed_flag(DDS::SUBSCRIPTION_MATCHED_STATUS, true);

DDS::DataReaderListener_var listener
= listener_for(DDS::SUBSCRIPTION_MATCHED_STATUS);

if (!CORBA::is_nil(listener.in())) {
DDS::DataReaderListener_var listener = listener_for(DDS::SUBSCRIPTION_MATCHED_STATUS);
if (listener) {
listener->on_subscription_matched(this, this->subscription_match_status_);

// Client will look at it so next time it looks the change should be 0
Expand All @@ -635,6 +630,13 @@ DataReaderImpl::remove_associations_i(const WriterIdSeq& writers,
if (this->monitor_) {
this->monitor_->report();
}

{
const RcHandle<DomainParticipantImpl> participant = participant_servant_.lock();
for (unsigned int i = 0; i < handles.length(); ++i) {
participant->return_handle(handles[i]);
}
}
}

void
Expand Down
10 changes: 10 additions & 0 deletions docs/devguide/quality_of_service.rst
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,10 @@ The ``max_samples`` member specifies the maximum number of samples a single data
The ``max_instances`` member specifies the maximum number of instances that a data writer or data reader can manage.
The ``max_samples_per_instance`` member specifies the maximum number of samples that can be managed for an individual instance in a single data writer or data reader.

.. warning::

Setting these to very high values may exhaust available memory.

Resources are used by the data writer to queue samples written to the data writer but not yet sent to all data readers because of backpressure from the transport.
Resources are used by the data reader to queue samples that have been received, but not yet read/taken from the data reader.

Expand Down Expand Up @@ -1226,6 +1230,10 @@ Reader Data Lifecycle QoS
The reader data lifecycle QoS policy controls the lifecycle of :term:`instance`\s managed by a :term:`DataReader`.

.. warning::

The default values (never purging) will result in increasing memory usage over time for each new instance received.

.. important::

This policy is :ref:`mutable <qos-changing>` and does not affect association.
Expand Down Expand Up @@ -1275,9 +1283,11 @@ This policy could, for example, permit a late-joining data writer to prolong the

The ``autopurge_nowriter_samples_delay`` controls how long the data reader waits before reclaiming resources once an instance transitions to the ``NOT_ALIVE_NO_WRITERS`` state.
By default, ``autopurge_nowriter_samples_delay`` is infinite.
Use :cfg:prop:`bit_autopurge_nowriter_samples_delay` to set this for :ref:`built-in data readers <bit>`.

The ``autopurge_disposed_samples_delay`` controls how long the data reader waits before reclaiming resources once an instance transitions to the ``NOT_ALIVE_DISPOSED`` state.
By default, ``autopurge_disposed_samples_delay`` is infinite.
Use :cfg:prop:`bit_autopurge_disposed_samples_delay` to set this for :ref:`built-in data readers <bit>`.

.. _qos-time-based-filter:
.. _quality_of_service--time-based-filter:
Expand Down
12 changes: 11 additions & 1 deletion docs/devguide/run_time_configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,16 @@ For example:
.. sec:: common

.. prop:: bit_autopurge_nowriter_samples_delay
:default: ``DURATION_INFINITE_SEC`` (disabled)

Sets the ``autopurge_nowriter_samples_delay`` value of the :ref:`quality_of_service--reader-data-lifecycle` for :ref:`built-in data readers <bit>`.

.. prop:: bit_autopurge_disposed_samples_delay
:default: ``DURATION_INFINITE_SEC`` (disabled)

Sets the ``autopurge_disposed_samples_delay`` value of the :ref:`quality_of_service--reader-data-lifecycle` for :ref:`built-in data readers <bit>`.

.. prop:: DCPSBidirGIOP=<boolean>
:default: ``1`` (enabled)

Expand Down Expand Up @@ -3331,7 +3341,7 @@ See :ref:`config-common` for options controlling the destination and formatting
The highest level logging is controlled by the general log levels listed in the following table.

.. list-table::
:header-rows: 1
:header-rows: 1

* - Level

Expand Down
9 changes: 9 additions & 0 deletions docs/news.d/dr-instance-handles.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
.. news-prs: 4784
.. news-start-section: Fixes
- Fixed a leak of instance handles in data readers that prevented :ref:`quality_of_service--reader-data-lifecycle` from properly purging data it was supposed to.
.. news-end-section
.. news-start-section: Documentation
- Documented the :cfg:prop:`bit_autopurge_nowriter_samples_delay` and :cfg:prop:`bit_autopurge_disposed_samples_delay` configuration properties.
.. news-end-section
2 changes: 1 addition & 1 deletion docs/news.d/relay-max-participants.rst
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.. news-prs: 4774
.. news-start-section: Additions
- Added -AdmissionMaxParticipantsRange to RtpsRelay options
- Added :option:`RtpsRelay -AdmissionMaxParticipantsRange`

- This option provides another mechanism for detecting load on each RtpsRelay instance

Expand Down

0 comments on commit fd485a9

Please sign in to comment.