Mir

Code review comment for lp://qastaging/~alan-griffiths/mir/doodle-in-TimeoutFrameDroppingPolicy-code

Revision history for this message
Alberto Aguirre (albaguirre) wrote :

This:
64+ if ([&]{ std::lock_guard<std::mutex> lock{mutex}; return --pending_swaps > 0; }())

should be:

bool update_alarm{false};
bool reschedule{false};
{
    std::lock_guard<std::mutex> lock{mutex};
    update_alarm = pending_swaps > 0;
    reschedule = update_alarm && --pending_swaps > 0;
}

if (update_alarm)
{
    if (reschedule)
        alarm->reschedule_in(timeout);
    else
        alarm->cancel();
}

This should address the following scenario:

Assume pending_swaps = 1

timeout occurs and Thread A executes alarm handler,

since pending_swaps = 1, after

37+ call_drop = pending_swaps > 0;
38+ reshedule = call_drop && --pending_swaps > 0;

pending_swaps = 0
call_drop = true
reshedule = false

Let's say Thread A gets pre-empted before invoking drop_frame()

Thread B calls swap_unblocked, now pending_swaps = -1

review: Needs Fixing

« Back to merge proposal