JdbiUtil: use concurrent data structure to store static state #2341

stevenschlansker · 2023-05-02T21:20:48Z

stevenschlansker · 2023-05-02T21:21:28Z

spring5/src/main/java/org/jdbi/v3/spring5/JdbiUtil.java


 import org.jdbi.v3.core.Handle;
 import org.jdbi.v3.core.Jdbi;
 import org.jdbi.v3.core.internal.UtilityClassException;
-import org.springframework.transaction.support.TransactionSynchronizationAdapter;


This class is deprecated for removal.

hgschmie · 2023-05-04T01:40:03Z

spring5/src/main/java/org/jdbi/v3/spring5/JdbiUtil.java

 import org.springframework.transaction.support.TransactionSynchronizationManager;

 /**
 * Utility for working with Jdbi and Spring transaction bound resources
 */
 public class JdbiUtil {
-    private static final Set<Handle> TRANSACTIONAL_HANDLES = new HashSet<>();
+    private static final Set<Handle> TRANSACTIONAL_HANDLES = Collections.newSetFromMap(new ConcurrentHashMap<>());


I am cool with this, but we did use CopyOnWriteSet in other places where concurrency was required.

hgschmie · 2023-05-04T01:40:31Z

RELEASE_NOTES.md

@@ -1,4 +1,5 @@
 # Unreleased
+  - spring5 JdbiUtil: fix thread safety


maybe add a reference to the issue that this addresses

stevenschlansker · 2023-05-04T02:09:08Z

Right, unless I am missing something, this is quite different. The copy on write map is useful with mostly immutable config where the most common operation is copying the map. In this case we never copy, we are just adding and removing elements to a single static collection, which is a better fit for a normal concurrent map. On May 3, 2023, at 6:40 PM, Henning Schmiedehausen ***@***.***> wrote: @hgschmie commented on this pull request. In spring5/src/main/java/org/jdbi/v3/spring5/JdbiUtil.java:

import org.springframework.transaction.support.TransactionSynchronizationManager;

/** * Utility for working with Jdbi and Spring transaction bound resources */ public class JdbiUtil { - private static final Set<Handle> TRANSACTIONAL_HANDLES = new HashSet<>(); + private static final Set<Handle> TRANSACTIONAL_HANDLES = Collections.newSetFromMap(new ConcurrentHashMap<>()); I am cool with this, but we did use CopyOnWriteSet in other places where concurrency was required. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>