JavaDocs for Client, WebTarget and Invocation should clearly say whether invocations MUST be thread-safe

[glassfishrobot]

JAX-RS 2.0 provides a Client API which is prepared for intense reuse of object instances of Client, WebTarget and Invocation. Unfortunately the JavaDocs of these classes do not say whether a caller can rely upon these instances are thread-safe. As a recent discussion on the user's mailing list has discovered, users abstain from the rist of concurrent usage of these instances as they fear risk of producing problems. Hence, the JAX-RS 2.1 specification should clearly point out (in PDF and / or JavaDocs) whether implementations of Client, WebTarget and Invocation MUST be thread-safe. This unambiguity would allow users to safely the same Client instance from different application threads for example.

Affected Versions

[2.1]

[glassfishrobot]

@glassfishrobot Commented
Reported by mkarg

[glassfishrobot]

@glassfishrobot Commented
Issue-Links:
is duplicated by
JAX_RS_SPEC-420
is related to
JAX_RS_SPEC-490
is related to
JAX_RS_SPEC-490

[glassfishrobot]

@glassfishrobot Commented
This issue was imported from java.net JIRA JAX_RS_SPEC-489

[glassfishrobot]

• Issue Imported From: https://github.com/jax-rs/api/issues/494
• Original Issue Raised By:@glassfishrobot
• Original Issue Assigned To: @glassfishrobot

[erik-brangs]

Is this planned for one of the next releases?

[andymc12]

@erik-brangs it is probably too late to go into the 3.0 release (and might not be allowed depending on if it changes behavior), but it could go into the 3.1 release. Would you be interested in open a pull request with the suggested changes to the javadocs?

[erik-brangs]

@andymc12 No, I'm not sure that I understand the API and the intention well enough to write the documentation.

I was asking because it would be nice if I could use one Client per application. However, it's still unclear to me how connection management would work in that case. The ClientBuilder API allows configuration of timeouts but it doesn't allow configuring number of concurrent connections or similar properties.

[mkarg]

(I am the original reporter of this issue)

I think we could address this in 3.1, and if nobody chimes in, I will provide a PR.

Before that, we should shortly agree upon some facts. Proposal:

• Client instances MUST be thread safe
• WebTarget instances MUST be thread safe
• SyncInvoke and AsyncInvoker instances MUST be thread safe
• Invocation instances MUST be thread safe
• Request and Response instances MUST be thread safe

while

• Invocation.Builder instances just MAY be thread safe

Committers (particulary vendors) WDYT?

[jansupol]

What does it mean to be thread-safe? Does it mean that thread A sets a property, and thread B sets the same property, replacing it for thread A? Or does it mean setting a property on thread A will create a new instance, so that thread B does not override it for thread A?

The same for instance for WebTarget, if thread A sets path, would it be replaced by thread B?

[mkarg]

"thread-safe" in general means that each thread MUST NOT see the changes of the other thread, so it looks like an isolated instance, hence allow sharing of one instance without risk of interfering. There MAY be particular exception from that rule where it makes sense.

The JavaDocs of WebTarget say: "Create a new WebTarget instance by appending path to the URI of the current target instance.". So if thread A and thread B both perform path(String), both get different instances, but the originally shared instance is unchanged.

[jansupol]

Right, the WebTarget does that, silly me. Do I get it right that we would have the same behavior for the Client, and instead of one Client, we would have one for each register and property, similarly to WebTarget?

What is the use-case for having Response thread-safe?

[mkarg]

I think what people expect is that configuration of the client will effectively change the same instance; it is one of the exceptions to the rule. Just the usage of the finally configured instance can be done multi-threaded. But maybe others chime in with different opinion?

Regarding Response I just want to be sure that if we could easily make it thread-safe then we should allow a multi-threaded access to the response. Just to be on the safe side if one day we finally see a good reason for multi-threaded processing of results. In fact, what people expect from a received Response IMHO is more like being immutable instead of actually being thread-safe. So a better wording would be to say "RECEIVED Requests and Responses are immutable on the receiver side".

[jansupol]

I think what people expect is that configuration of the client will effectively change the same instance; it is one of the exceptions to the rule.

Precisely, it is an exception to each thread MUST NOT see the changes of the other thread, so it looks like an isolated instance. Jersey always provided a new instance of a WebTarget upon setting a property; it is aligned with the rest of the WebTarget methods, and it is somehow expected, that doing:

   target = ....
   target.property().request().get()
   target.request().get()

would behave once with the property and second without the property. That's the whole point of holding the WebTarget in hand to me. So Jersey users would expect the configuration effectively change different instances.

Regarding the Response, I see there is no need of having it thread-safe. It is hardly an immutable object, given there is a readEntity method.

[spericas]

"thread-safe" in general means that each thread MUST NOT see the changes of the other thread, so it looks like an isolated instance, hence allow sharing of one instance without risk of interfering. There MAY be particular exception from that rule where it makes sense.

This is a stronger condition than thread safety, which is really about guaranteeing code that is "free of race conditions". Object immutability and copy-on-writes is one particular approach. Just stating that something needs to be thread safe does not imply that threads must not see each others updates.

Before getting to a PR, I think it would be useful to go over some use cases for all these types. Thread safety usually has a cost associated with it, so a case needs to be made as to why paying such a cost is acceptable. As an example, it's really hard for me to see the need for Response to be thread safe.

[mkarg]

@spericas Agreed in general, but in case we agree that Response MAY (instead of MUST) be thread-safe there is no additional cost, at least for Jersey -- as per @jansupol's description what it already works like. :-)

[mkarg]

@andymc12 @ronsigal @deki What is the opinion of IBM, Red Hat and Apache?

[spericas]

@spericas Agreed in general, but in case we agree that Response MAY (instead of MUST) be thread-safe there is no additional cost, at least for Jersey -- as per @jansupol's description what it already works like. :-)

How does it help a Jakarta REST developer to know that a class MAY be thread safe? It means checking an implementation's docs to see if something is thread safe, and that is already the status quo. We MUST avoid MAY.

[jansupol]

Agreed in general, but in case we agree that Response MAY (instead of MUST) be thread-safe there is no additional cost, at least for Jersey -- as per @jansupol's description what it already works like. :-)

I was merely talking about WebTarget implementation in Jersey. Not Response.

[mkarg]

So is there a counter proposal basing on the outcome of this discussion so far?

[spericas]

@spericas Agreed in general, but in case we agree that Response MAY (instead of MUST) be thread-safe there is no additional cost, at least for Jersey -- as per @jansupol's description what it already works like. :-)

How does it help a Jakarta REST developer to know that a class MAY be thread safe? It means checking an implementation's docs to see if something is thread safe, and that is already the status quo. We MUST avoid MAY.

I agree it is not ideal, but you're not addressing the cost factor of making everything thread safe (there's no free lunch). Also, saying nothing or saying that it is implementation dependent is not the same. If I read the latter, I expect the implementation docs to clarify it; otherwise, I don't.

[mkarg]

I agree it is not ideal, but you're not addressing the cost factor of making everything thread safe (there's no free lunch). Also, saying nothing or saying that it is implementation dependent is not the same. If I read the latter, I expect the implementation docs to clarify it; otherwise, I don't.

Actually I do not want to imply costs. My proposal is not to change existing implementations to make them thread-safe but to find out the minimum status-quo of existing implementations and simply describe that in the spec. Just to let app developers have a safe base on which they can trust.

[spericas]

I agree it is not ideal, but you're not addressing the cost factor of making everything thread safe (there's no free lunch). Also, saying nothing or saying that it is implementation dependent is not the same. If I read the latter, I expect the implementation docs to clarify it; otherwise, I don't.

Actually I do not want to imply costs. My proposal is not to change existing implementations to make them thread-safe but to find out the minimum status-quo of existing implementations and simply describe that in the spec. Just to let app developers have a safe base on which they can trust.

I see, I have no problem with that. We need to put together a table then and let implementers fill it out.

[mkarg]

I am sorry that I did not find the time in 2021 to set up this table. I propose we move this issue to the next release so it does not stand in the way for 3.1. Everybody fine with this proposal?