CopyOnWriteArraySet is jdk1.6 On Tue, Jun 21, 2011 at 4:09 PM, Martijn Dashorst <[email protected]> wrote: > On Tue, Jun 21, 2011 at 12:52 PM, Juergen Donnerstag > <[email protected]> wrote: >> mind you providing a test case to investigate why it's failing. I >> don't want to say it is the reason, but may be WiQuery is using it the >> wrong way and their devs just didn't detect it yet. > > The newly introduced state/assertions try to prevent > ConcurrentModificationExceptions when adding listeners to the target > while the listeners are being processed. While this is a laudable > goal, it fails when something that just works without causing > ConcurrentModificationExceptions now suddenly is forbidden. > > That said, WiQuery does the following to ensure that a component does > the JQuery initialization dance. The javascript js is something like > "$('id').activatePlugin();" > > AjaxRequestTarget ajaxRequestTarget = (AjaxRequestTarget) requestHandler; > ajaxRequestTarget.addListener(new IListener() { > public void onAfterRespond(Map<String, Component> map, > IJavaScriptResponse response) { > response.addJavaScript(js); > } > > public void onBeforeRespond(Map<String, Component> map, > AjaxRequestTarget target) { > // Do nothing > } > }); > > This is (ultimately) called from AjaxRequestTarget#renderHead(). > > Now I am quite positive that this is a bit of over-engineering/left > overs from 1.4 days on WiQuery's part, according to me the following > should also work: > > AjaxRequestTarget ajaxRequestTarget = (AjaxRequestTarget) requestHandler; > requestHandler.getHeaderResponse().renderOnDomReadyJavaScript(js); > > Given that WiQuery probably does something stupid (the simplified code > suggests such), I do think that the current implementation of > restricting additions to the listeners is too strict. > > There are a couple of things we can change: > 1. relax the assertions such that they only cover the cases where > things actually go wrong, or > 2. use CopyOnWriteArraySet instead of HashSet to make iteration safe > even when things get added to the set, or > 3. ...? > > Number 2 sounds like a good idea, but might introduce errors where > someone is adding a listener which doesn't get processed since the > iterator works on a previous copy—hard to debug, but quite easy to > track (keep a previous size and compare/assert after the loop). It > also consumes more memory during request processing. > > I agree with the issue that a ConcurrentModificationException is less > descriptive than asserting that things are set in stone. So I'd like > to opt for number 1 and relax the assertions to just those spots where > things go wrong, *and* properly document it instead of relying on > runtime exceptions that may/may not occur. > > Martijn >
-- Martin Grigorov jWeekend Training, Consulting, Development http://jWeekend.com
