Skip to content

Commit

Permalink
Improve synchronization on the R2 RequestContext local attributes.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jpstewart committed May 12, 2023
1 parent a912f01 commit 49ae010
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
version=29.42.1
version=29.42.2
group=com.linkedin.pegasus
org.gradle.configureondemand=true
org.gradle.parallel=true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object> localAttrs)
{
_localAttrs = localAttrs;
_localAttrs = Collections.synchronizedMap(localAttrs);
}

/**
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 49ae010

Please sign in to comment.