Opened 5 months ago
Last modified 4 months ago
#2771 new defect
UDP EstablishmentManager tries to resend expired messages
Reported by: | jogger | Owned by: | zzz |
---|---|---|---|
Priority: | minor | Milestone: | undecided |
Component: | router/transport | Version: | 0.9.47 |
Keywords: | Cc: | ||
Parent Tickets: | Sensitive: | no |
Description
processExpired() calls failed(), which in turn goes to afterSend(). There the message gets requeued, is found as invalid in OutNetMessagePool?.add() and finally arrives at OutboundMessageRegistry?.unregisterPending() with a null reply selector.
It is clear that add() misses the reply selector null check that is always prepended to unregisterPending() elsewhere.
(rtime > 0 && rtime + OB_MESSAGE_TIMEOUT ⇐ now)" used in handleOutbound(). |
To verify throw NPE in unregisterPending() for a null reply selector and wait hours/days.
Subtickets
Change History (3)
comment:2 Changed 4 months ago by
Above patch in 7ff58573c1cce2cd56e7bb541af6d5f8183bc975 to be 0.9.47-5
leaving ticket open to await clarification on the other points
comment:3 Changed 4 months ago by
In Paragraph 3 I wanted to express that it makes little sense to retry a connection that already failed while establishing. So I would imagine a shortcut somewhere to drop everything related instead of calling methods aimed at retransmitting on working connections.
Yeah calling unregisterPending() with a null reply selector appears to be harmless, but would be better to not do it, here's a diff for that:
The idea is that a timeout for an individual transport could be less than the overall message expiration, so it could be resent by another transport. But if the overall message has expired, we shouldn't. Not sure I understand what OP is proposing in the 3rd paragraph.