From 2ca7edcf3fcbc82e1d42341c70e916baf5bd4b8a Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Fri, 25 Sep 2026 15:31:54 +0200 Subject: [PATCH] Fix operator precedence bug in datagram ACK timeout calculation In `MemoryConfigurationService`, `1 << (flags & 0x0F) * 1000` evaluated the multiplication before the bitwise shift due to Java operator precedence. This caused the exponent $N$ to shift by $(N \times 1000) \pmod{32}$, yielding incorrect timeout intervals (e.g., 256 ms instead of 2 s for $N=1$, and 65.5 s instead of 4 s for $N=2$). Parenthesize the shift operation to compute $(1 \ll N) \times 1000\text{ ms}$ per the OpenLCB Datagram Transport Standard, and add corresponding unit tests. --- .../MemoryConfigurationService.java | 11 +++++++++-- .../MemoryConfigurationServiceTest.java | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/org/openlcb/implementations/MemoryConfigurationService.java b/src/org/openlcb/implementations/MemoryConfigurationService.java index 3ca8f116..768f0f73 100644 --- a/src/org/openlcb/implementations/MemoryConfigurationService.java +++ b/src/org/openlcb/implementations/MemoryConfigurationService.java @@ -527,6 +527,14 @@ public void run() { retryTimer.schedule(tt, timeout); } + static int computeTimeout(int flags) { + int timeoutExp = flags & 0x0F; + if (timeoutExp == 0) { + return 3000; // no timeout specified, 3 seconds is default + } + return (1 << timeoutExp) * 1000; // 2^N seconds to msec + } + // invoked when retryTimer times out private void timeoutRetry(final McsRequestMemo memo) { logger.log(Level.FINE, "timeoutRetry entry"); @@ -556,8 +564,7 @@ public void handleSuccess(int flags) { ((flags & DatagramService.FLAG_REPLY_PENDING) != 0)) { // Leave the memo in the pending, will wait for reply datagram. logger.fine("rcvd RequestWithReplyDatagram with flags "+flags); - int timeout = 1 << (flags & 0x0F) * 1000; // to msec - if ((flags & 0x0F) == 0) timeout = 3000; // no timeout specified, 3 seconds is default + int timeout = computeTimeout(flags); logger.fine(" and timeout "+timeout+", restarting"); restartTimeout(memo, timeout); return; diff --git a/test/org/openlcb/implementations/MemoryConfigurationServiceTest.java b/test/org/openlcb/implementations/MemoryConfigurationServiceTest.java index f8f28e49..209ff54c 100644 --- a/test/org/openlcb/implementations/MemoryConfigurationServiceTest.java +++ b/test/org/openlcb/implementations/MemoryConfigurationServiceTest.java @@ -702,4 +702,20 @@ public void handleAddrSpaceData(NodeID dest, int space, long hiAddress, long low } + @Test + public void testComputeTimeout() { + // Exponent 0: default 3000 ms + Assert.assertEquals(3000, MemoryConfigurationService.computeTimeout(0)); + Assert.assertEquals(3000, MemoryConfigurationService.computeTimeout(DatagramService.FLAG_REPLY_PENDING | 0)); + + // Exponents 1..4: 2^N seconds in milliseconds + Assert.assertEquals(2000, MemoryConfigurationService.computeTimeout(DatagramService.FLAG_REPLY_PENDING | 1)); + Assert.assertEquals(4000, MemoryConfigurationService.computeTimeout(DatagramService.FLAG_REPLY_PENDING | 2)); + Assert.assertEquals(8000, MemoryConfigurationService.computeTimeout(DatagramService.FLAG_REPLY_PENDING | 3)); + Assert.assertEquals(16000, MemoryConfigurationService.computeTimeout(DatagramService.FLAG_REPLY_PENDING | 4)); + + // Max exponent 15: 2^15 seconds in milliseconds + Assert.assertEquals(32768000, MemoryConfigurationService.computeTimeout(DatagramService.FLAG_REPLY_PENDING | 15)); + } + }