On Wed, May 9, 2012 at 3:42 PM, Fabio Mancinelli <[email protected]> wrote: > On Wed, May 9, 2012 at 3:37 PM, Thomas Mortagne > <[email protected]> wrote: >> On Wed, May 9, 2012 at 3:32 PM, Fabio Mancinelli >> <[email protected]> wrote: >>> On Wed, May 9, 2012 at 3:18 PM, Marius Dumitru Florea >>> <[email protected]> wrote: >>>> On Wed, May 9, 2012 at 12:12 PM, Thomas Mortagne >>>> <[email protected]> wrote: >>>>> On Wed, May 9, 2012 at 11:06 AM, Thomas Mortagne >>>>> <[email protected]> wrote: >>>>>> On Tue, May 8, 2012 at 7:12 PM, Marius Dumitru Florea >>>>>> <[email protected]> wrote: >>>>>>> Hi Fabio and devs, >>>>>>> >>>>>>> I found a serious concurrency issue in the REST server module while >>>>>>> debugging the instability of the Extension Manager when >>>>>>> extensions.xwiki.org repository is used (default case) . The Extension >>>>>>> Manager UI searches extensions using REST and very often it gets 500 >>>>>>> HTTP response code. See http://jira.xwiki.org/browse/XWIKI-7773 for >>>>>>> instance. The server log from xwiki.org shows that the real cause is: >>>>>>> >>>>>>> May 8, 2012 5:09:14 PM org.restlet.engine.application.StatusFilter >>>>>>> doHandle >>>>>>> WARNING: Exception or error caught in status service >>>>>>> java.util.ConcurrentModificationException >>>>>>> at >>>>>>> java.util.AbstractList$Itr.checkForComodification(AbstractList.java:372) >>>>>>> at java.util.AbstractList$Itr.next(AbstractList.java:343) >>>>>>> at >>>>>>> org.xwiki.rest.XWikiSetupCleanupFilter.afterHandle(XWikiSetupCleanupFilter.java:73) >>>>>>> >>>>>>> See the full stacktrace http://pastebin.com/hnFSuwem . >>>>>>> >>>>>>> The problem is related to the way "releasable components" are managed. >>>>>>> I debugged locally both XWikiSetupCleanupFilter [1] and >>>>>>> ComponentsObjectFactory and here's what I discovered: >>>>>>> >>>>>>> * org.restlet.Context.getCurrent() is shared across HTTP requests >>>>>> >>>>>> What I don't understand is that the Context is stored in a ThreadLocal >>>>>> (see org.restlet.Context) so I don't see how it can be shared across >>>>>> HTTP requests. Or maybe it's reused by two consecutive requests ? >>>>> >>>>> Actually it could be that Restlet put the same context in all the >>>>> request using Context#setCurrent(). >>>>> >>>> >>>>> We should maybe use the XWiki Execution context which has been made >>>>> for use cases like that to be safe. >>>> >>>> Indeed. >>>> >>>> The code that manually releases REST resource components that have a >>>> per-lookup instantiation strategy looks to me like a workaround for >>>> the fact that we don't have a per-request (or per-execution) 'release' >>>> strategy (i.e. components that are automatically released at the end >>>> of the request/execution). >>>> >>> That's exactly this. To avoid memory leaks due to the allocations in >>> the ComponentsObjectFactory of per-lookup components that would never >>> be released otherwise. >>> Re the thread issues, I thought that the context was per-request but >>> AFAIU from what you're saying it's not the case. >>> >>> Maybe we could put a thread local providing the actual list in the >>> context instead of putting the list directly. >> >> Either it's a Restlet bug that should be fixed or we should use >> Execution context IMO. >> > Execution context is better, right. > I don't remember why I didn't use it originally, if there was a > blocking reason or just an oversight.
If Restlet Context was tied to the thread it would sounds nicer to me to use it so what you did make sense. > > -Fabio > >>> >>> -Fabio >>> >>>> Thanks, >>>> Marius >>>> >>>>> >>>>>> >>>>>>> * as a consequence, restlet context attributes are shared across HTTP >>>>>>> request >>>>>>> * RELEASABLE_COMPONENT_REFERENCES context attribute is thus also shared >>>>>>> * while a thread iterates this list in XWikiSetupCleanupFilter another >>>>>>> thread can add a component to the list in ComponentsObjectFactory >>>>>>> * the list grows indefinitely because XWikiSetupCleanupFilter only >>>>>>> releases the components; it doesn't remove them from the list >>>>>>> * older instances are re-released >>>>>>> * since the list keeps references to older instances these instances >>>>>>> can't be garbage collected >>>>>>> >>>>>>> Could someone more knowledgeable on the REST module (especially >>>>>>> Restlet) review my findings? >>>>>>> >>>>>>> Thanks, >>>>>>> Marius >>>>>>> >>>>>>> [1] >>>>>>> https://github.com/xwiki/xwiki-platform/blob/master/xwiki-platform-core/xwiki-platform-rest/xwiki-platform-rest-server/src/main/java/org/xwiki/rest/XWikiSetupCleanupFilter.java >>>>>>> [2] >>>>>>> https://github.com/xwiki/xwiki-platform/blob/master/xwiki-platform-core/xwiki-platform-rest/xwiki-platform-rest-server/src/main/java/org/xwiki/rest/ComponentsObjectFactory.java >>>>>>> _______________________________________________ >>>>>>> devs mailing list >>>>>>> [email protected] >>>>>>> http://lists.xwiki.org/mailman/listinfo/devs >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Thomas Mortagne >>>>> >>>>> >>>>> >>>>> -- >>>>> Thomas Mortagne >>>>> _______________________________________________ >>>>> devs mailing list >>>>> [email protected] >>>>> http://lists.xwiki.org/mailman/listinfo/devs >>>> _______________________________________________ >>>> devs mailing list >>>> [email protected] >>>> http://lists.xwiki.org/mailman/listinfo/devs >>> _______________________________________________ >>> devs mailing list >>> [email protected] >>> http://lists.xwiki.org/mailman/listinfo/devs >> >> >> >> -- >> Thomas Mortagne >> _______________________________________________ >> devs mailing list >> [email protected] >> http://lists.xwiki.org/mailman/listinfo/devs > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs -- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
