amichair commented on PR #43: URL: https://github.com/apache/aries-rsa/pull/43#issuecomment-2134837600
I've opened and fixed https://issues.apache.org/jira/browse/ARIES-2130 based on this PR. Note that the link and quote above about LinkedHashMap specifically refer to access-ordered linked hash maps, in which a get operation can modify it. However, the default for LinkedHashMap when using the no-args constructor, as in our case, is to use insertion order rather than access order, so there shouldn't be any modifications caused by the get operations afaik. As for replacing keySet iteration followed by get with a single values iteration, I applied that as well though it has nothing to do with concurrency, just for readability and performance. In general, one should iterate over the keySet when only keys are needed, iterate over values when only values are needed, and iterate over entrySet when both are needed as a pair. Finally, for the concurrency fix, there are a couple of methods that were accessing the exportsMap (and the lists within it that also need to be accessed concurrently) that weren't synchronized, so I added it as in your fix, which I think should solve the problem. However, the commit above also synchronized two of the ExportRegistrationHolder methods that have nothing to do with this map, and without a stack trace or additional info or explanation it's hard to tell if there was indeed another problem encountered there or this was just added 'just in case' but it is unclear why. I'm reluctant to add them since unnecessary synchronization can also lead to deadlocks, so they should be used with caution. If you have additional info on this please open a separate issue with the details. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@aries.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
