[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Matej Novotny updated WELD-2545 Weld / WELD-2545 ConcurrentValidator.validateBeanNames and thread-safety Change By: Matej Novotny Status: Pull Request Sent Resolved Resolution: Done Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Issue was automatically transitioned when Matej Novotny created pull request #1882 in GitHub Weld / WELD-2545 ConcurrentValidator.validateBeanNames and thread-safety Change By: Matej Novotny Status: Open Pull Request Sent Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Michael Rasmussen commented on WELD-2545 Re: ConcurrentValidator.validateBeanNames and thread-safety Matej Novotny a SetMultimap backed by a ConcurrentHashMap should fix this issue yeah. Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Matej Novotny edited a comment on WELD-2545 Re: ConcurrentValidator.validateBeanNames and thread-safety So what we could do is to use a concurrent version of {{SetMultimap}} which is backed by {{ConcurrentHashMap}}. Not sure how exactly resizing works there, but I suppose it is accounted for.There was also another change in the affected code as a part of WELD-2322 where a new set was created before invoking {{Beans.removeDisabledBeans}} - this bit shouldn't matter though as the set will be synchronized and access by at most one thread at a time . Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Matej Novotny commented on WELD-2545 Re: ConcurrentValidator.validateBeanNames and thread-safety I created a PR with the aforementioned changes. Michael Rasmussen since testing this is tricky, could you please take a look at it and tell me if you think this will do the trick for you? Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Matej Novotny edited a comment on WELD-2545 Re: ConcurrentValidator.validateBeanNames and thread-safety So what we could do is to use a concurrent version of {{SetMultimap}} which is backed by {{ConcurrentHashMap}}. Not sure how exactly resizing works there, but I suppose it is accounted for.There was also another change in the affected code as a part of WELD-2322 where a new set was created before invoking {{ Validator Beans . validateBeanName() removeDisabledBeans }}. Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Matej Novotny updated an issue Weld / WELD-2545 ConcurrentValidator.validateBeanNames and thread-safety Change By: Matej Novotny Git Pull Request: https://github.com/weld/core/pull/1882 Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Matej Novotny updated an issue Weld / WELD-2545 ConcurrentValidator.validateBeanNames and thread-safety Change By: Matej Novotny Fix Version/s: 3.1.0.Beta1 Fix Version/s: 3.1.0.Final Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Matej Novotny assigned an issue to Matej Novotny Weld / WELD-2545 ConcurrentValidator.validateBeanNames and thread-safety Change By: Matej Novotny Assignee: Matej Novotny Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Matej Novotny commented on WELD-2545 Re: ConcurrentValidator.validateBeanNames and thread-safety So what we could do is to use a concurrent version of SetMultimap which is backed by ConcurrentHashMap. Not sure how exactly resizing works there, but I suppose it is accounted for. There was also another change in the affected code as a part of WELD-2322 where a new set was created before invoking Validator.validateBeanName(). Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Michael Rasmussen edited a comment on WELD-2545 Re: ConcurrentValidator.validateBeanNames and thread-safety The concurrent iterator calls validateBeanName(name, namedAccessibleBeans, accessibleNamespaces, beanManager)The first line of Validator.validateBeanName calls namedAccessibleBeans.get(name) <-- here a potential resize can happen because of the computeIfAbsent. Edit: we've encountered this during our automatic testing of our (JRebel's) integration with weld, but due to it basically having to basically be an exact number of beans, and the flaky nature of concurrency, sometimes hard to reproduce. Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Matej Novotny edited a comment on WELD-2545 Re: ConcurrentValidator.validateBeanNames and thread-safety Hello [~maldivia]I am not sure I can see the problem here. Have you encountered an actual error with this? - Looking at the code of {{ConcurrentValidator.validateBeanNames}} what is actually parallel is just going over the key set itself via iterator. - - Implementation of [{{AbstractMultimap.keySet()}} returns an {{ImmutableSet.copyOf(map.keySet());}}|https://github.com/weld/core/blob/master/impl/src/main/java/org/jboss/weld/util/collections/AbstractMultimap.java#L106], so there is no resizing happening. -I was blind, I can see that {{Validator.validateBeanName()}} still operates on the set. Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Michael Rasmussen commented on WELD-2545 Re: ConcurrentValidator.validateBeanNames and thread-safety The concurrent iterator calls validateBeanName(name, namedAccessibleBeans, accessibleNamespaces, beanManager) The first line of Validator.validateBeanName calls namedAccessibleBeans.get(name) <-- here a potential resize can happen because of the computeIfAbsent. Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Matej Novotny commented on WELD-2545 Re: ConcurrentValidator.validateBeanNames and thread-safety Hello Michael Rasmussen I am not sure I can see the problem here. Have you encountered an actual error with this? Looking at the code of ConcurrentValidator.validateBeanNames what is actually parallel is just going over the key set itself via iterator. Implementation of AbstractMultimap.keySet() returns an ImmutableSet.copyOf(map.keySet());, so there is no resizing happening. Add Comment This message was sent by Atlassian Jira (v7.12.1#712002-sha1:609a505) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2545) ConcurrentValidator.validateBeanNames and thread-safety
Title: Message Title Michael Rasmussen created an issue Weld / WELD-2545 ConcurrentValidator.validateBeanNames and thread-safety Issue Type: Bug Affects Versions: 3.0.5.Final Assignee: Unassigned Components: Bootstrap and Metamodel API Created: 19/Nov/18 8:28 PM Priority: Major Reporter: Michael Rasmussen In the method validateBeanNames in org.jboss.weld.bootstrap.ConcurrentValidator, the variable namedAccessibleBeans is a SetMultimap, but not a concurrent one, thus basically a wrapper for HashMap>. If the number of keys in the map is higher than the threshold for when the underlying HashMap resizes itself (for instance size=13 for current OpenJDK defaults), calling SetMultimap.get() will cause a resize, as the underlying implementation uses HashMap.computeIfAbsent, which resizes the map if size is above threshold regardless of if the key is absent or not. This is an issue since in the method this map is used concurrently, iterating over existing keys, thus under the assumption that such a resize will not happen, as it can have weird consequences. Also see http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-November/056736.html