liferay-faces-bridge thread safe? ConcurrentModificationException

[jjiiu ', geändert vor 9 Jahren.]

Liferay-faces-bridge 3.1.4-ga5, LR 6.1.30, tomcat 6, java 7

See https://gist.github.com/anonymous/505d986e09665f4d9590. Should this be a jira ticket? Did not find a related ticket. Looks like a bridge bug to me.

[David H Nebinger, geändert vor 9 Jahren.]

Wow, that doesn't make any sense.

Looking at the code for 3.1.4 ga5 on line 79 is just the for () construct for iterating over the loop of saved context attributes, it's not modifying the list at all.

I'm sure Neil's here somewhere, hopefully he'll have some input...

[Neil Griffin, geändert vor 9 Jahren.]

Hi David,

Thanks for looking at the code. Juan Gonzalez will be helping with this issue.

Neil

[Tobias Liefke, geändert vor 9 Jahren.]

Hi David,

I'd like to point out, that a ConcurrentModificationException is not thrown if you modify a collection while iterating, but if you iterate while modifying it.

Check this code:

List<string> tests = new ArrayList&lt;&gt;(Arrays.asList("Test1", "Test2"));
for (String test : tests) { // &lt;- Will throw a ConcurrentModificationException here
  tests.remove(test);
}
</string>

That you modify the collection in the same thread or in a different one doesn't matter - as long as it was modified during the runtime of the loop.

So for BridgeRequestScopeCompatImpl.restoreJSF2FacesContextAttributes::

• either the savedFacesContextAttributes are the same as the currentFacesContextAttributes - no thread safety problem then
• or restoreJSF2FacesContextAttributes is called by two threads for the same portlet at the same time. I don't know if there is any synchronization guard that prevents this...

[David H Nebinger, geändert vor 9 Jahren.]

It's gotta be line 84 where currentFacesContextAttributes.put() statement is...

[Tobias Liefke, geändert vor 9 Jahren.]

If you mean the statement that modifies the collection while iterating:
If it is a threading issue, it could be anywhere where the collection is modified - e.g. savedFacesContextAttributes.clear() as well

[Juan Gonzalez, geändert vor 9 Jahren.]

Hi,

how could you reproduce this issue? Just by one request? Multiple request? Is this portlet added to a page multiple times?

Did you follow the configuration for jsf2-spring portlets like this: https://github.com/liferay/liferay-faces/tree/3.1.x/demos/bridge/jsf2-spring-portlet

Would be great if you could add your Spring beans into that demo portlet and try to reproduce again.

Thanks.

[jjiiu ', geändert vor 9 Jahren.]

I was unable to reproduce the issue. What I did was click the submit fast twice. You can see the jpa provider logging stuff and my test method running twice. Clicking fast after that didn't help hehe it was the only time, not surprising though. Yeah reproducing this will be almost impossible without some framework to spam requests, it may even then come down to luck. Imo looks like two requests got served at the same time by two different threads ;)

[EL Fine]: sql: 2015-02-19 14:17:03.602--ServerSession(1425301616)--Connection(1591498020)--Thread(Thread[http-8080-47,5,main])--SELECT ID, NAME FROM Test
[EL Fine]: sql: 2015-02-19 14:17:03.602--ServerSession(1425301616)--Connection(1077965623)--Thread(Thread[http-8080-40,5,main])--SELECT ID, NAME FROM Test
14:17:03,607 ERROR [HelloBean:36] 0
14:17:03,607 ERROR [HelloBean:36] 0

[Neil Griffin, geändert vor 9 Jahren.]

jjiiu ':

I was unable to reproduce the issue. What I did was click the submit fast twice. You can see the jpa provider logging stuff and my test method running twice. Clicking fast after that didn't help hehe it was the only time, not surprising though. Yeah reproducing this will be almost impossible without some framework to spam requests, it may even then come down to luck. Imo looks like two requests got served at the same time by two different threads ;)

I think your analysis is correct in that this will be very difficult to reproduce. I also think that the two HTTP POST requests would have to come from the same portlet, on the same portal page, and that very very quickly. If this is a concern, then I recommend that you research the typical techniques for preventing double form submission such as disabling the button as soon as it is clicked.

[jjiiu ', geändert vor 9 Jahren.]

Alright so you are saying it has to come from the same portlet instance as in this won't happen in the real world with double submission disabled?

[Neil Griffin, geändert vor 9 Jahren.]

jjiiu ':

Alright so you are saying it has to come from the same portlet instance as in this won't happen in the real world with double submission disabled?

I could be wrong, but that is the conclusion that I came to after researching the problem. At the time of this writing, Liferay Faces Bridge has been used in production environments for over 2 years and this issue has never been reported.

[jjiiu ', geändert vor 9 Jahren.]

Neil Griffin:

jjiiu ':

Alright so you are saying it has to come from the same portlet instance as in this won't happen in the real world with double submission disabled?

I could be wrong, but that is the conclusion that I came to after researching the problem. At the time of this writing, Liferay Faces Bridge has been used in production environments for over 2 years and this issue has never been reported.

Yes yes sounds sensible. Although this kind of stuff is very hard to hit and easy to write off as non issue because of the infrequency. Why not take the source stop the first thread in to the loop fire multiple requests that are not stopped and release first thread and see what happens. I do understand this is unnecessary if the code analysis is uhuh sensible, just an idea! Anyhow i'm treating it as a nonissue.