Re: [PR] Changed simple synchronization to ReentrantReadWriteLock [tomcat]
ChristopherSchultz commented on PR #712: URL: https://github.com/apache/tomcat/pull/712#issuecomment-2032266991 Thank you for this PR, though I was already working on a patch and the unit tests pass, so I'm going to commit my patch instead of yours. I will credit you for the idea in the changelog, however. I like my patch better than yours primarily because you are placing the call to `lock.lock()` within the `try` block, and it should be placed before the `try` block instead. Also, I use two separate `Lock` objects instead of calling `ReentrantReadWriteLock.writeLock()` or `.readLock()` each time, which makes it easier to read the code to see that the correct lock is being used. I _could_ just add commits to your PR to make it work, but the work is already done on my end so I'm gonna commit mine. -- 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...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Changed simple synchronization to ReentrantReadWriteLock [tomcat]
ChristopherSchultz closed pull request #712: Changed simple synchronization to ReentrantReadWriteLock URL: https://github.com/apache/tomcat/pull/712 -- 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...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[PR] Changed simple synchronization to ReentrantReadWriteLock [tomcat]
LupusMKW opened a new pull request, #712: URL: https://github.com/apache/tomcat/pull/712 Due to https://github.com/apache/tomcat/commit/4f33be682fda02a616baa0fd9b4965d248cfe1c1 our application does no longer work. The commit improved the situation with respect to concurrent mutability of the Services array. Previously, it was possible to read the array while it was being modified concurrently. Unfortunately, it can now happen that Tomcat runs into a deadlock if a starting application attempts to gather information from the Services array (which all our applications do). The proposed change replaces the simple lock with a ReentrantReaderWriter lock, so that readers can read the Services array and only writers are blocked. -- 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...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
