From 49ae010da14594802f85669b4296ebeb2fd1bf13 Mon Sep 17 00:00:00 2001 From: John Stewart Date: Wed, 26 Apr 2023 12:54:21 -0700 Subject: [PATCH] Improve synchronization on the R2 `RequestContext` local attributes. It is possible to cause a `ConcurrentModificationException` in the `RequestContext` because we synchronize it by using `Collections.synchronizedMap()`, which only synchronizes individual reads/writes. When making a `RequestContext` copy we must iterate over the collection, allowing other threads to interrupt it by modifying the map during the operation. This fix synchronizes the whole copy operation as well. The default mutex used for synchronizing in a `SynchronizedMap` is `this`, so synchronizing on the map itself guards against modification. --- CHANGELOG.md | 6 +++++- gradle.properties | 2 +- .../main/java/com/linkedin/r2/message/RequestContext.java | 8 +++++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6aa2d2a7c0..e8cc30d9b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ and what APIs have changed, if applicable. ## [Unreleased] +## [29.42.2] - 2023-05-11 +- Fix synchronization on `RequestContext` to prevent `ConcurrentModificationException`. + ## [29.42.1] - 2023-05-11 - Add support for returning location of schema elements from the PDL schema encoder. @@ -5465,7 +5468,8 @@ patch operations can re-use these classes for generating patch messages. ## [0.14.1] -[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.42.1...master +[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.42.2...master +[29.42.2]: https://github.com/linkedin/rest.li/compare/v29.42.1...v29.42.2 [29.42.1]: https://github.com/linkedin/rest.li/compare/v29.42.0...v29.42.1 [29.42.0]: https://github.com/linkedin/rest.li/compare/v29.41.12...v29.42.0 [29.41.12]: https://github.com/linkedin/rest.li/compare/v29.41.11...v29.41.12 diff --git a/gradle.properties b/gradle.properties index 3702b1dcc2..8f88ec9791 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -version=29.42.1 +version=29.42.2 group=com.linkedin.pegasus org.gradle.configureondemand=true org.gradle.parallel=true diff --git a/r2-core/src/main/java/com/linkedin/r2/message/RequestContext.java b/r2-core/src/main/java/com/linkedin/r2/message/RequestContext.java index 707622e383..717f427352 100644 --- a/r2-core/src/main/java/com/linkedin/r2/message/RequestContext.java +++ b/r2-core/src/main/java/com/linkedin/r2/message/RequestContext.java @@ -54,12 +54,14 @@ public RequestContext() */ public RequestContext(RequestContext other) { - _localAttrs = Collections.synchronizedMap(new HashMap<>(other._localAttrs)); + synchronized (other._localAttrs) { + _localAttrs = Collections.synchronizedMap(new HashMap<>(other._localAttrs)); + } } private RequestContext(Map localAttrs) { - _localAttrs = localAttrs; + _localAttrs = Collections.synchronizedMap(localAttrs); } /** @@ -117,7 +119,7 @@ public RequestContext clone() public boolean equals(Object o) { return (o instanceof RequestContext) && - ((RequestContext)o)._localAttrs.equals(this._localAttrs); + ((RequestContext)o)._localAttrs.equals(this._localAttrs); } @Override