Opened 2 weeks ago

Last modified 11 days ago

#2609 new defect

PeerState.java: locked_shouldSend() violates send order

Reported by: jogger Owned by: zzz
Priority: minor Milestone: undecided
Component: router/transport Version: 0.9.42
Keywords: Cc:
Parent Tickets: Sensitive: no

Description

In case we have already postponed a state because it did not fit one of the send windows, locked_shouldSend() returns ShouldSend?.NO. This causes a subsequent send to either go through invalidating the nextSendTime for older blocked sends or be also postponed with a then inappropriate nextSendTime.

As we have called finishMessages() in between possibly freeing up the send window, we should retry the oldest first, transforming the first if:

if (state.getNextSendTime() <= now || state.getPushCount() == 0) {

Subtickets

Change History (2)

comment:1 Changed 11 days ago by zzz

I'm confused. The send time is not updated if the method returns ShouldSend?.NO. So there's no need for a change here.

comment:2 Changed 11 days ago by jogger

The send time has been updated before. That is the logic we discussed before in #2506. That logic sets a future send time for a rejected state. If ShouldSend?.NO is returned upon checking this state during the next round robin, the rejected state is currently skipped when send time > now.

Then allocateSend() pulls the next state off _outboundQueue and checks that. As finishMessages() may have cleared b/w in between that next state may go through and is sent before the delayed state.

Note: See TracTickets for help on using tickets.