Fix crash when disable_positive_acks is enable and the remote reader is best-effort [17338] (backport #3324)

src/cpp/rtps/writer/StatefulWriter.cpp

@@ -435,15 +435,6 @@ void StatefulWriter::unsent_change_added_to_history(
                 }
                 );
 
-        if (should_be_sent)
-        {
-            flow_controller_->add_new_sample(this, change, max_blocking_time);
-        }
-        else
-        {
-            periodic_hb_event_->restart_timer(max_blocking_time);
-        }
-
         if (disable_positive_acks_)
         {
             auto source_timestamp = system_clock::time_point() + nanoseconds(change->sourceTimestamp.to_ns());
@@ -454,6 +445,17 @@ void StatefulWriter::unsent_change_added_to_history(
             ack_event_->update_interval_millisec((double)duration_cast<milliseconds>(interval).count());
             ack_event_->restart_timer(max_blocking_time);
         }
+
+        // After adding the CacheChange_t to flowcontroller, its pointer cannot be used because it may be removed
+        // internally before exiting the call. For example if the writer matched with a best-effort reader.
+        if (should_be_sent)
+        {
+            flow_controller_->add_new_sample(this, change, max_blocking_time);
+        }
+        else
+        {
+            periodic_hb_event_->restart_timer(max_blocking_time);
+        }
     }
     else
     {

test/blackbox/common/BlackboxTestsAcknackQos.cpp

@@ -160,3 +160,37 @@ TEST(AcknackQos, NotRecoverAfterLosingCommunicationWithDisablePositiveAck)
     // Block reader until reception finished or timeout.
     ASSERT_EQ(reader.block_for_all(std::chrono::seconds(1)), 0u);
 }
+
+/*!
+ * @test Regresion test for Github #3323.
+ */
+TEST(AcknackQos, DisablePositiveAcksWithBestEffortReader)
+{
+    PubSubReader<HelloWorldPubSubType> reader(TEST_TOPIC_NAME);
+    PubSubWriter<HelloWorldPubSubType> writer(TEST_TOPIC_NAME);
+
+    writer.keep_duration({2, 0});
+    writer.reliability(eprosima::fastrtps::RELIABLE_RELIABILITY_QOS);
+    writer.durability_kind(eprosima::fastrtps::VOLATILE_DURABILITY_QOS);
+    writer.init();
+
+    reader.keep_duration({1, 0});
+    reader.reliability(eprosima::fastrtps::BEST_EFFORT_RELIABILITY_QOS);
+    reader.init();
+
+    ASSERT_TRUE(reader.isInitialized());
+    ASSERT_TRUE(writer.isInitialized());
+
+    // Wait for discovery.
+    writer.wait_discovery();
+    reader.wait_discovery();
+
+    std::list<HelloWorld> data = default_helloworld_data_generator();
+    reader.startReception(data);
+    // Send data
+    writer.send(data);
+    // In this test all data should be sent.
+    ASSERT_TRUE(data.empty());
+    // Block reader until reception finished or timeout.
+    reader.block_for_all();
+}