Re: [xwiki-devs] XWikiContext cloning

Hi devs, We all know that the old XWikiContext is a burden that must still be carried around, in order to access any non-componentized functionality. The problem is that a context object is not supposed to be used by more than one thread. Example of non-threadsafe parts of the context are: - the Hibernate session - the request and response objects, cleared by the container - the velocity context - the XClass cache - the current wiki name - and maybe others This single-thread restriction is generally acceptable, since most of the code is executed in the single-threaded request-response workflow. Yet, some plugins execute in separate threads, for example the Lucene indexer and the scheduler plugin, and they need their XContext object. The current strategy is to clone the context and remove some of the dangerous elements listed above. This is not good, since: - it has to be done in every plugin that creates threads (duplication) - adding more non-threadsafe things to the context requires that all these plugins are changed - some non-threadsafe things might not yet be identified, and they surface sometimes as unidentified bugs - some things cannot be cleared from the outside (for example the XClass cache, which is a private member of the context) There are several solutions to this problem: 1. Override the clone() method to remove non-threadsafe elements from the cloned context. Pro: removes duplication Pro: establishes a safe clone method for all the codebase Con: some unsafe things might be overlooked, surfacing from time to time in rare thread inter-weaving. 2. Override the clone() method to create a blank context and only copy what needs to be part of any context. Pro: same as above, but also eliminates all possible non-threadsafe elements. Con: We might overlook something that needs to be part of the context. The advantage over option 1 is that this is always reproducible, and a simple stack trace is enough to spot the problem, unlike multithreaded issues. Con: We might introduce a regression, this needs to be tested well. 3. Override the clone() method to just eliminate non-threadsafe things that are unaccessible from outside (the XClass cache is the only one I see). Pro: keeps the current behavior, reducing the risk of regressions. Con: Doesn't really solve the problem. I'd go with option 2. Any other opinions?