PaulGale commented on issue #2271:
URL: https://github.com/apache/activemq/issues/2271#issuecomment-5146973406
_**Note: this comment was generated by Claude Code after troubleshooting
this issue.**_
## Additional diagnosis: the regression is the advertised JMS version, not
the missing feature
I hit this independently and dug into why it appears only in 6.3.0.
Short version: **delivery delay was never implemented, in 6.2.x either.**
What changed in 6.3.0 is what ActiveMQ *claims* about itself, and
`pooled-jms` is behaving correctly in response to that claim. That means this
can be fixed without implementing #1717 at all.
### `getDeliveryDelay()` is byte-for-byte identical in 6.2.8 and 6.3.0
A full `javap -c` diff of `ActiveMQMessageProducerSupport` between
`activemq-client` 6.2.8 and 6.3.0 shows **no differences whatsoever**. Both
throw:
```
public long getDeliveryDelay() throws jakarta.jms.JMSException;
0: new #33 // class java/lang/UnsupportedOperationException
4: ldc #40 // String getDeliveryDelay() is not supported
6: invokespecial #37 // UnsupportedOperationException."<init>":(String)V
9: athrow
```
So the method is not the variable. Bisecting `activemq-client` against an
otherwise fixed dependency set:
| `activemq-client` | Result |
|---|---|
| 6.2.6 | passes |
| 6.2.8 | passes |
| 6.3.0 | fails on the first send |
### What actually changed: `ActiveMQConnectionMetaData`
| | `getJMSVersion()` | `getJMSMajorVersion()` |
|---|---|---|
| 6.2.8 | `"1.1"` | `1` |
| 6.3.0 | `"3.1"` | `3` |
Introduced by commit `6871226ff1` — *"feat(tck): Add Jakarta Messaging 3.1
TCK with some fixes"* (PR #1712, sub-PR **#1718**). Its stated rationale:
> ActiveMQ reports JMS version as 1.1 (JMS 1.1 era). Since the project now
uses jakarta.jms
> (Jakarta Messaging 3.1), the metadata must match.
Respectfully, this conflates two different things: the **API artifact
namespace**
(`javax.jms` → `jakarta.jms`) and the **specification level implemented**.
Compiling against the `jakarta.jms` 3.1 API says nothing about implementing JMS
2.0 semantics — and `ConnectionMetaData` is not documentation, it is a
**capability-negotiation API** that libraries branch on at runtime.
### Why that breaks `pooled-jms` specifically
`JmsPoolMessageProducer`'s constructor snapshots producer defaults, but
**guards the JMS 2.0 getters behind the provider's own claim**:
```
101: iconst_2 // major = 2
102: iconst_0 // minor = 0
103: invokevirtual isJMSVersionSupported:(II)Z // does the provider support
JMS 2.0?
106: ifeq 119 // NO -> skip, return
109: getDeliveryDelay() // YES -> call it
119: return
```
`isJMSVersionSupported` resolves from `connection.getMetaData()`. So:
- Against **6.2.8**: provider answers major = 1, `1 >= 2` is false, the call
is **never made**.
- Against **6.3.0**: provider answers major = 3, `3 >= 2` is true, the call
**is** made and throws.
`pooled-jms` is doing exactly the right thing — asking before calling an
optional API. The regression is that ActiveMQ now answers "yes" to a question
it cannot honour.
For contrast, TIBCO EMS advertises JMS 3.0 **and implements**
`deliveryDelay`, so the identical `pooled-jms` code path works there. Two ways
to be safe: implement it, or don't claim it. 6.3.0 does neither.
### Two things that make this worse in practice
**The secondary symptom is badly misleading.** Because producer creation
fails, pooled sessions are never returned to the pool, so after a short while
the visible error becomes `java.util.NoSuchElementException: Pool exhausted`.
Anyone debugging from that symptom will be looking at pool sizing, not at JMS
capability metadata. It cost me a while.
**`UnsupportedOperationException` is unchecked**, while the JMS interface
declares these methods `throws JMSException`. Client code written to the
interface contract cannot catch it, so it propagates as a raw runtime failure
through pooling, `JmsTemplate` and framework error handling alike.
Scope note: because `pooled-jms` is managed by `spring-boot-dependencies`,
this reaches ordinary Spring Boot + ActiveMQ applications with JMS pooling
enabled. It reproduces with **no application code referencing delivery delay
anywhere** — `pooled-jms` calls it on its own initiative.
### The claim is also contradicted inside this repository
- `activemq-unit-tests/.../jms2/ActiveMQJMS2ContextTest.java` (lines
282–314) *asserts* that
`get/setDeliveryDelay`, `createSharedConsumer`,
`createSharedDurableConsumer` and async `send`
with a `CompletionListener` all throw `UnsupportedOperationException`.
- The Classic documentation describes support as **"Partial Jakarta
Messaging 3.1"**, and states clients *"will not support all JMS 2.0
functionality and will throw an `UnsupportedOperationException` … such as those
for async send with a `CompletionListener`, sending messages with a delivery
delay, and using shared topic consumers."*
- On that page **AMQ-8320 (Delivery Delay), targeted at 6.3.0, is still
open** — so 6.3.0 shipped the claim while the implementation it was targeted
alongside did not land.
Worth noting the TCK module added in `6871226ff1` runs only behind an opt-in
`-Prun-tck` profile and is not part of the default build, so the 3.1 claim is
not continuously verified. Its exclusion list (`ts.jtx`) contains a single
entry, attributed to a TCK bug — nothing acknowledging delivery delay or shared
consumers. I could not run the TCK myself, so I am inferring rather than
asserting that these were never exercised.
### The claim covers more than delivery delay
Eight methods still throw on `main`, all now advertised as supported:
- `getDeliveryDelay`, `setDeliveryDelay` — in both
`ActiveMQMessageProducerSupport`
(lines 58–71) and `ActiveMQProducer` (lines 248–254, the JMS 2.0
simplified `JMSProducer` API)
- `createSharedConsumer` ×2, `createSharedDurableConsumer` ×2,
`createSharedConnectionConsumer`, `createSharedDurableConnectionConsumer` —
AMQ-8323
Delivery delay is simply the first one a library happened to probe. The
shared-consumer methods are armed behind the same claim and will surface the
same way for any consumer that gates on advertised version.
### Suggested resolutions
Listed cheapest first. Any one resolves the regression; **none requires
implementing #1717.**
**1. Degrade delivery delay to its documented default instead of throwing.**
JMS defines the default delivery delay as `0`, meaning "no delay", so a
provider that only supports immediate delivery can report `0` truthfully.
Concretely: have `getDeliveryDelay()` return the stored value (default `0`);
accept `setDeliveryDelay(0)` as a no-op; and continue to throw only for a
**non-zero** delay, so a delay a caller genuinely requested is never silently
dropped. This unblocks every capability-probing consumer while keeping the
honest failure where it matters. It needs the same treatment in
`ActiveMQProducer` for the simplified API, and two existing assertions in
`ActiveMQJMS2ContextTest` would need updating to match.
**2. Narrow the advertised capability** until AMQ-8320 / AMQ-8323 land —
i.e. report the level actually implemented. Lowest risk and it restores 6.2.x
behaviour exactly, but it puts ActiveMQ back to advertising JMS 1.1 while
shipping the `jakarta.jms` 3.1 API, which is what motivated #1718 in the first
place.
**3. Complete AMQ-8320 and AMQ-8323** before advertising 3.1. Correct, but
by far the largest piece of work, and unnecessary to unblock affected users now.
My own preference is **(1)**, because it is small, spec-defensible, keeps
the 3.1 claim that #1718 wanted, and fixes the whole class of
capability-probing consumers rather than just `pooled-jms`.
I'm happy to open a PR with a failing test plus the fix for whichever
direction you prefer — I didn't want to send code before there was agreement on
the approach.
### Reproducer
No application code touches delivery delay:
1. Wrap an `ActiveMQConnectionFactory` in a `pooled-jms`
`JmsPoolConnectionFactory`.
2. Send any message through it (directly, or via Spring's `JmsTemplate`).
3. With `activemq-client` 6.2.8 the send succeeds. With 6.3.0 it throws
`UnsupportedOperationException: getDeliveryDelay() is not supported` at
producer construction,
and every subsequent send fails until the pool reports exhaustion.
### One suggestion for pooled-jms (separate project)
Independently of the above, `JmsPoolMessageProducer` could treat these
optional getters defensively — catch `UnsupportedOperationException` and fall
back to `0` rather than letting producer creation fail. That would harden it
against any provider that advertises a JMS level without implementing every
optional API. Happy to raise that upstream if useful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
