kennknowles commented on code in PR #25913:
URL: https://github.com/apache/beam/pull/25913#discussion_r1179537188
##########
sdks/java/extensions/protobuf/src/main/java/org/apache/beam/sdk/extensions/protobuf/ProtoCoder.java:
##########
@@ -275,7 +279,7 @@ public ExtensionRegistry getExtensionRegistry() {
final Set<Class<?>> extensionHostClasses;
// Transient fields that are lazy initialized and then memoized.
- private transient ExtensionRegistry memoizedExtensionRegistry;
+ private transient volatile ExtensionRegistry memoizedExtensionRegistry;
Review Comment:
Marking it volatile is redundant with synchronization, right?
##########
sdks/java/extensions/protobuf/src/main/java/org/apache/beam/sdk/extensions/protobuf/ProtoCoder.java:
##########
@@ -247,17 +247,21 @@ public Set<Class<?>> getExtensionHosts() {
*/
public ExtensionRegistry getExtensionRegistry() {
if (memoizedExtensionRegistry == null) {
- ExtensionRegistry registry = ExtensionRegistry.newInstance();
- for (Class<?> extensionHost : extensionHostClasses) {
- try {
- extensionHost
- .getDeclaredMethod("registerAllExtensions",
ExtensionRegistry.class)
- .invoke(null, registry);
- } catch (IllegalAccessException | InvocationTargetException |
NoSuchMethodException e) {
- throw new IllegalStateException(e);
+ synchronized (this) {
Review Comment:
We chatted off the PR and I understand this is to remove the benign race so
that concurrency analyzers are happy.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
