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. -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
