Re: [jetty-users] Threading model (was Re: Jetty 12 schedule?_
On 11/10/22, 5:45 PM, "jetty-users on behalf of Greg Wilkins" wrote: >To my knowledge that cannot happen at least not with normal > threads. Perhaps in future with virtual threads, then threadlocals might > be more of a problem, but their usage will be optional in Jetty. Virtual threads don't change things if they're implemented the way they're supposed to be, they intended to maintain the existing contracts for threading in Java, and that includes thread-locals. The whole point of that work in fact is to get people to stop coding up asynchronous designs, because multi-threading is a lot simpler. The async behaviors are supposed to be moved under the covers. -- Scott ___ jetty-users mailing list jetty-users@eclipse.org To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jetty-users
Re: [jetty-users] Threading model (was Re: Jetty 12 schedule?_
On Thu, 10 Nov 2022 at 23:53, Cantor, Scott wrote: > > What we're concerned about is the drumbeat that a non-async-using servlet > request that's just executing methods on objects via the stack could start > out on one thread and switch threads in the middle. And I just don't see > how that's remotely possible or sane. And if it is, the spec went to a very > bad place and a lot of software is screwed, not just ours. > To my knowledge that cannot happen at least not with normal threads. Perhaps in future with virtual threads, then threadlocals might be more of a problem, but their usage will be optional in Jetty. -- Greg Wilkins CTO http://webtide.com ___ jetty-users mailing list jetty-users@eclipse.org To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jetty-users
Re: [jetty-users] Threading model (was Re: Jetty 12 schedule?_
>Problems start to occur when a request/response is wrapped in > another object that abstracts away from the request response paradigm > and just looks like some dataful object (which just happens to be backed > by data a request). Such objects can easily be put into persistent data > structures that have no lifecycle relationship to the original request. Yes. We actually changed a lot of properties on singletons from taking an HttpServletRequest to Supplier precisely because while *we* know we're not injecting an actual request object but rather a proxy for one, it's not obvious from the code. That was not good. But they were never being given an actual request. Only a proxy for retrieving the "current" one. The hitch is how one can know what the current one is across a call stack and that's where we used thread-locals. But we have never had any lifecycle problems, because we populate the thread-locals from a filter and the filter clears them during the unwind, and it's never caused any trouble. What we're concerned about is the drumbeat that a non-async-using servlet request that's just executing methods on objects via the stack could start out on one thread and switch threads in the middle. And I just don't see how that's remotely possible or sane. And if it is, the spec went to a very bad place and a lot of software is screwed, not just ours. -- Scott ___ jetty-users mailing list jetty-users@eclipse.org To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jetty-users
Re: [jetty-users] Threading model (was Re: Jetty 12 schedule?_
On Thu, 10 Nov 2022 at 12:29, Cantor, Scott wrote: > >The proper way is to use the request / response objects as passed to > > you, and not hold onto them. > > That would imply that one has to immediate extract every possible bit from > them and construct a new façade for the data before one's servlet method > even calls another object. That's a pretty huge step backward, if true. > Scott, it is typical to handle requests with multi classes, potentially calling deep into a code base. Often these calls will accept the request and/or response as an argument and that is a reasonable style as it makes it very clear that the call is part of a request/response calling cycle. Problems start to occur when a request/response is wrapped in another object that abstracts away from the request response paradigm and just looks like some dataful object (which just happens to be backed by data a request). Such objects can easily be put into persistent data structures that have no lifecycle relationship to the original request. So the antipattern is to facade a request into a non-request like API. If your facades do look like requests and their lifecycle is apparent, then that is a reasonable abstraction. regards -- Greg Wilkins CTO http://webtide.com ___ jetty-users mailing list jetty-users@eclipse.org To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jetty-users
Re: [jetty-users] Threading model (was Re: Jetty 12 schedule?_
>The assumption that you'll be passed a HttpServletRequest and/or > HttpServletResponse to these actions isn't 100% true. Ok, I can see that. >Take for example the Async I/O events. Which we will never, ever call/use/allow. >There are similar things elsewhere in the spec. Most of them are in the same bucket, we don't do that kind of thing. > Keep in mind that there is no provision in the Servlet spec for notifying > of the lifecycle of the HttpServletRequest or HttpServletResponse > objects. Understood, but they have to exist for the lifetime of the call to the servlet dispatch method, absent use of other container services that interrupt the processing chain of that method, no? Otherwise it's madness since we couldn't even make it work by passing them around if we wanted to. >The proper way is to use the request / response objects as passed to > you, and not hold onto them. That would imply that one has to immediate extract every possible bit from them and construct a new façade for the data before one's servlet method even calls another object. That's a pretty huge step backward, if true. > As a servlet endpoint, you are expected to get what you need from the > request (headers, body, etc), formulate a response (status code, > headers, etc) and then produce a body. Of course, but that is never (outside trivial samples) one class. It's a chain of many objects and methods doing work. You don't pass the interfaces into every method. And even if you did pass them into every method in a synchronous call chain, you're implying that wouldn't even help. And that seems impossible since I can't even see how you would ever be in the middle of the code to interfere with it. Again, there is no use of async anything whatsoever (once we get control), and there never will be. For exactly this reason. >If you need to track things between dispatches of the same exchange, > it's usually a good idea to use the request attributes to hold onto objects > for the scope of that one request. If I have no HttpServletRequest, how would I possibly put or get them? That doesn't change the situation, really. >Don't forget about normal redispatch behavior. That's worth more study, especially the error case, but that's likely past the point we'd be doing anything unusual depending on these interfaces. But it is a point we need to consider. >Imagine if the APIs contained the request/response objects > everywhere, which ones do we give you if wrapping occurs? (eg: if a > Filter wrapped the request, and added the listener, then the Servlet > wrapped again, and then that listener needed to fire, do we give you the > servlet wrapped request or the filter wrapped request?) Yes, I can see why they had to do things the way they did for the Async APIs. Which we don't use. -- Scott ___ jetty-users mailing list jetty-users@eclipse.org To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jetty-users
Re: [jetty-users] Threading model (was Re: Jetty 12 schedule?_
> Is it fair to assume that the only way a request could possibly cross threads like that is if there's also an actually explicit entry point into one's application logic by which the container is actually invoking a standard interface that is defined to accept the HttpServletRequest/Response? The assumption that you'll be passed a HttpServletRequest and/or HttpServletResponse to these actions isn't 100% true. Take for example the Async I/O events. Those are listeners. https://javadoc.io/static/jakarta.servlet/jakarta.servlet-api/5.0.0/jakarta/servlet/ReadListener.html https://javadoc.io/static/jakarta.servlet/jakarta.servlet-api/5.0.0/jakarta/servlet/WriteListener.html You are not expected to be manipulating / accessing / referencing the request and/or response at this point in the process. That should have happened long before you even engaged these listeners. There are similar things elsewhere in the spec. Keep in mind that there is no provision in the Servlet spec for notifying of the lifecycle of the HttpServletRequest or HttpServletResponse objects. If a method existed to know when the original HttpServletRequest and HttpServletResponse was / is / has been ended it's lifecycle (and optionally been recycled by the container), you could possible write code to hold onto the request/response objects safely (but realities of concurrency programming means that this kind of event can only occur AFTER it's done, which is often too late to turn things off) > In other words, I take your point that people can build code that leverages various advanced async/etc. features in the servlet spec, and we don't necessarily know that's happening, but it has to *be* some explicit invocation of a container technology that in turn is going to call back in with the "proper" servlet request/response, right? The proper way is to use the request / response objects as passed to you, and not hold onto them. This even includes objects like HttpSession and the ServletInputStream / ServletOutputStream. As a servlet endpoint, you are expected to get what you need from the request (headers, body, etc), formulate a response (status code, headers, etc) and then produce a body. The async processing exists to delay / suspend actions to later, but that also results in a container managed thread for the AsyncContext, one that the container is aware of and knows the scope/lifecycle of. The async I/O for requests expects that you've gathered what you need from the request url line / headers / etc before you start. The async I/O for responses expects that you've decided what you are going to send, and are setting up to send the body of the response. If you need to track things between dispatches of the same exchange, it's usually a good idea to use the request attributes to hold onto objects for the scope of that one request. > So it's difficult to understand how we could possibly be passed in a different instance than the original one the servlet dispatch received since there's nowhere for it to occur. Don't forget about normal redispatch behavior. An example of this is if you use the RequestDispatcher, this will be a wrapped or altered request/response objects depending on your mode of use (include vs forward). You can also have an Error being processed via a redispatch using the DispatcherType.ERROR (which can appear to be a different request/response, but in reality is a spec mandated cleanup of the request/response before redispatch). In both of those cases, a request attribute works great to track behavior throughout the entire request lifecycle (even redispatch). > Basically it feels like for this to break, something has to call an API that also involves passing in a callback interface that itself has to receive the request/response back. As I showed above, not all listeners / interfaces have the request/response params given to you, the API was created to respect the Filter Chain, DispatcherTypes, and Request/Response Wrappers along with the normal HTTP lifecycle (eg: you can't change response status code or headers if the HttpServletResponse.isCommitted() is true) Imagine if the APIs contained the request/response objects everywhere, which ones do we give you if wrapping occurs? (eg: if a Filter wrapped the request, and added the listener, then the Servlet wrapped again, and then that listener needed to fire, do we give you the servlet wrapped request or the filter wrapped request?) Joakim Erdfelt / joa...@webtide.com On Wed, Nov 9, 2022 at 5:22 PM Cantor, Scott wrote: > >It's a general anti-pattern to hold onto, use, reference a > > HttpServletRequest or HttpServletResponse object outside of the > > dispatch from the container. > > One more question about this I guess... > > Is it fair to assume that the only way a request could possibly cross > threads like that is if there's also an actually explicit entry point into > one's application logic by which the container is actually invoking a >
[jetty-users] Threading model (was Re: Jetty 12 schedule?_
>It's a general anti-pattern to hold onto, use, reference a > HttpServletRequest or HttpServletResponse object outside of the > dispatch from the container. One more question about this I guess... Is it fair to assume that the only way a request could possibly cross threads like that is if there's also an actually explicit entry point into one's application logic by which the container is actually invoking a standard interface that is defined to accept the HttpServletRequest/Response? In other words, I take your point that people can build code that leverages various advanced async/etc. features in the servlet spec, and we don't necessarily know that's happening, but it has to *be* some explicit invocation of a container technology that in turn is going to call back in with the "proper" servlet request/response, right? Otherwise how could one ever know which instance of the request/response interfaces to act on? All we do is implement servlets (or make use of them) that accept a servlet dispatch call in and respond out. Nothing else in our design implements any servlet APIs that can accept a request/response in a different way. So it's difficult to understand how we could possibly be passed in a different instance than the original one the servlet dispatch received since there's nowhere for it to occur. That's why we've struggled to grasp what the risk is as this has come up at times. I fully appreciate that you're speaking from a container/generality perspective here and that in the general case what you're saying is obviously true. And yes, it's an anti-pattern because it makes assumptions that aren't generally true. I'm strongly suspecting they are, however, still true for us (and frankly for the vast majority of traditional apps that just implement Servlet). Basically it feels like for this to break, something has to call an API that also involves passing in a callback interface that itself has to receive the request/response back. -- Scott ___ jetty-users mailing list jetty-users@eclipse.org To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jetty-users
