Fixed error sending more than 256 fragments <1.9.x> [7403]

[richiware]

The async thread was not awaken to send the next unsent fragments after sending the first 256. This PR fixes this.

[richiware]

Build status:

• Linux
• Mac
• Windows

[dirk-thomas]

I can confirm that this patch makes it work for the use case where the number of UDP packets a data sample is split to is getting high. Thank you for the quick fix!

[dirk-thomas]

@richiware Please see the related feature request: #976.

[dirk-thomas]

Can you please clarify if this bug has always existed or if not in which version it was introduced?

[MiguelCompany]

Apart from the styling issues I have put in comments, I see the following:

• We need a blackbox test for this use case.
• I don't think this is the correct way to do it. These changes will make the async thread wakeup whenever there is more than one fragment to be sent, as the wake_up call is inside the while loop.

There are two use cases here:

1. The flow controllers limit the number of fragments sent. In this case the flow controllers will wake-up the writer when the bandwidth is recovered (inside a timed event)
2. There are more than 256 pending fragments

Proposed solution:

1. Detect if flow controllers limited the traffic, by checking if the number of items in changesToSend is reduced
2. Change method send_any_unsent_changes to something like

std::lock_guard<RecursiveTimedMutex> guard(mp_mutex);

bool flow_controllers_limited = false;
while(!unsent_changes_.empty() && !flow_controllers_limited)
{
    RTPSWriterCollector<ReaderLocator*> changesToSend;
    for (const ChangeForReader_t& unsentChange : unsent_changes_)
    {
        ...
    }

    if (there_are_remote_readers_ || !fixed_locators_.empty())
    {
        size_t n_items = unsent_changes_.size();

        // Flow controllers processing
        ...

        flow_controllers_limited = n_items != unsent_changes_.size();
    }
    else   // This was not done before and could be very important!
    {
        unsent_changes_.clear();
    }
}

[MiguelCompany]

Can you please clarify if this bug has always existed or if not in which version it was introduced?

I think it was introduced on #486, where we changed FragmentNumberSet_t from a std::set to a fixed sized bitmap. So it has probably been there since release 1.8.0.

@richiware Could you address the proposed changes, and then backport the fix to 1.8.x?

[richiware]

Build status:

• Linux
• Mac
• Windows

[MiguelCompany]

Failed tests are not related