Hi Mark, tested on a lighter sample reproducing the issue pretty easily (real one would need 8.5) and your patch fixed it keeping the sample working fine
Romain Manni-Bucau @rmannibucau <https://twitter.com/rmannibucau> | Blog <https://blog-rmannibucau.rhcloud.com> | Old Wordpress Blog <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber <http://www.tomitribe.com> | JavaEE Factory <https://javaeefactory-rmannibucau.rhcloud.com> 2016-09-03 20:38 GMT+02:00 Mark Thomas <ma...@apache.org>: > On 03/09/2016 18:08, Romain Manni-Bucau wrote: > > This is an interesting quote cause i understand as it is implemented in > > tomcat but also means any usage would potentially lead to deadlocks or > > container integration to ensure thread safety is done right making in > both > > cases the spec unusable or hard to use from a user perspective. Should it > > be pushed back to the spec? > > It wouldn't hurt for the EG to clarify exactly what they meant by > "...will not take effect until...". If it is intended that it might > include blocking the thread making the call, stating that explicitly > would be good. > > Meanwhile, try the following patch and let me know how you get on. It > achieves the same result (hopefully) without the blocking. The unit > tests pass but I want to think about it some more (and add some > comments) before committing. > > http://people.apache.org/~markt/patches/2016-09-03- > async-refactoring-tc9-v1.patch > > Mark > > > > > > Le 3 sept. 2016 18:28, "Mark Thomas" <ma...@apache.org> a écrit : > > > >> On 02/09/2016 15:47, Romain Manni-Bucau wrote: > >>> ~business code (simplified): > >>> > >>> public void myBusiness(AsyncResponse wrapperOfAsyncContext) { > >>> new Thread() { > >>> @Override > >>> public void run() { > >>> wrapperOfAsyncContext.synchronizedDispatch(); > >>> } > >>> }.start(); > >>> wrapperOfAsyncContext.synchronizedMethod(); > >>> } > >>> > >>> If timing is good (and encountered enough to say it is in real life) > then > >>> both method call will lock cause: > >>> > >>> 1. for the user thread: the dispatch call will call > >> pauseNonContainerThread > >>> to wait the myBusiness thread to end (actually a bit more but you get > the > >>> idea) > >>> 2. synchronizedMethod will lock on synchronizedDispatch > >>> > >>> so synchronizedMethod is lock on synchronizedDispatch which is lock on > >>> the pauseNonContainerThread of the caller/http thread so both threads > are > >>> locked. > >> > >> Hmm. > >> > >> The relevant text from the Servlet spec is (for both dispatch() and > >> complete()) is: > >> > >> "If any of the dispatch methods are called before the > >> container-initiated dispatch that called startAsync has returned to the > >> container, then the call will not take effect until after the > >> container-initiated dispatch has returned to the container." > >> > >> The critical question is what does "...will not take effect until..." > mean. > >> > >> Currently, Tomcat implements this as "...will block until...". I'll look > >> into what it would take to implement this as "...note the request and > >> execute it once...". Whether or not we change this will depend a lot on > >> how complicated the change would be. > >> > >> Mark > >> > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > >> For additional commands, e-mail: dev-h...@tomcat.apache.org > >> > >> > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
