Repository: open-feature/java-sdk
Pull request #131 (2022-10-07 04:01:34)

fix!: add rw locks to client/api, hook accessor name
This PR adds multi-read/single-write locks to both the global singleton and the client. Particularly it addresses problems relating to the providers/hooks being mutated while an evaluation is occurring. This is especially important since some providers have provider hooks, and if we dont lock, it's possible hooks from proivder1 run on provider2 if an interleaving thread switched the provider.

To make things cleaner and less error-prone, I've created a little wrapper so we can use `try-with-resources` blocks.

This way, locking can be done like:

```
    public void addHooks(Hook... hooks) {
        try (AutoCloseableLock __ = rwLock.writeLockAutoCloseable()) {
            this.apiHooks.addAll(Arrays.asList(hooks));
        }
    }
```

...with no need to explicitly close the lock.

I'm going to write some tests in the morning for this before I mark it as ready.

Closes: https://github.com/open-feature/java-sdk/issues/64

codecov-commenter: # [Codecov](https://codecov.io/gh/open-feature/java-sdk/pull/131?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) Report
> Merging [#131](https://codecov.io/gh/open-feature/java-sdk/pull/131?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) (d692312) into [main](https://codecov.io/gh/open-feature/java-sdk/commit/da7a662334c61a931297ecefcc5a5202b4179ab7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) (da7a662) will **increase** coverage by `0.10%`.
> The diff coverage is `100.00%`.

```diff
@@             Coverage Diff              @@
##               main     #131      +/-   ##
============================================
+ Coverage     91.21%   91.31%   +0.10%     
- Complexity      177      182       +5     
============================================
  Files            19       20       +1     
  Lines           387      403      +16     
  Branches         23       23              
============================================
+ Hits            353      368      +15     
  Misses           23       23              
- Partials         11       12       +1     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `91.31% <100.00%> (+0.10%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/open-feature/java-sdk/pull/131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) | Coverage Δ | |
|---|---|---|
| [.../main/java/dev/openfeature/sdk/OpenFeatureAPI.java](https://codecov.io/gh/open-feature/java-sdk/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature#diff-c3JjL21haW4vamF2YS9kZXYvb3BlbmZlYXR1cmUvc2RrL09wZW5GZWF0dXJlQVBJLmphdmE=) | `95.65% <100.00%> (-4.35%)` | :arrow_down: |
| [...in/java/dev/openfeature/sdk/OpenFeatureClient.java](https://codecov.io/gh/open-feature/java-sdk/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature#diff-c3JjL21haW4vamF2YS9kZXYvb3BlbmZlYXR1cmUvc2RrL09wZW5GZWF0dXJlQ2xpZW50LmphdmE=) | `97.75% <100.00%> (+0.10%)` | :arrow_up: |
| [.../internal/AutoCloseableReentrantReadWriteLock.java](https://codecov.io/gh/open-feature/java-sdk/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature#diff-c3JjL21haW4vamF2YS9kZXYvb3BlbmZlYXR1cmUvc2RrL2ludGVybmFsL0F1dG9DbG9zZWFibGVSZWVudHJhbnRSZWFkV3JpdGVMb2NrLmphdmE=) | `100.00% <100.00%> (ø)` | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature)

justinabrahms: Looks good so far. Interested to see the tests.
toddbaert: I _almost_ have some tests working - but it's Canadian Thanksgiving and I have other obligations! I will pickup next week.
toddbaert: @justinabrahms I've added tests.
toddbaert: I've added a bit more granularity to the locking.
toddbaert: @justinabrahms @agentgonzo 

I think I'm finished with this now. It's analogous to https://github.com/open-feature/dotnet-sdk/pull/79 with a few java differences changes.

Initially I was locking in the evaluation, but I don't think that's necessary or beneficial. The current locks mainly protect from errors due to out of order issues and collection mutation during enumeration.

Additionally, I've made changes to access the provider just once in the evaluation logic. This prevents things from becoming incoherent if the provider is charged mid evaluation.
toddbaert: @justinabrahms one other thing worth pointing out ... I've changed the names of the accessors for hooks on the client and API, in order to be more consistent with addhooks, which is the same for both the API and client.

If you disagree with that, I can revert.
justinabrahms: @toddbaert Seems fine to me, but I changed your PR title to denote the breaking change.

Patch:
diff --git a/pom.xml b/pom.xml
index e15949d..443da5c 100644
--- a/pom.xml
+++ b/pom.xml
@@ -164,6 +164,21 @@
   <build>
     <plugins>
 
+      <plugin>
+        <groupId>org.codehaus.mojo</groupId>
+        <artifactId>build-helper-maven-plugin</artifactId>
+        <version>3.3.0</version>
+        <executions>
+          <execution>
+            <phase>validate</phase>
+            <id>get-cpu-count</id>
+            <goals>
+              <goal>cpu-count</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
+
       <plugin>
         <groupId>org.cyclonedx</groupId>
         <artifactId>cyclonedx-maven-plugin</artifactId>
@@ -231,6 +246,9 @@
           <argLine>
             ${surefireArgLine}
           </argLine>
+          <!-- fork a new JVM to isolate test suites, especially important with singletons -->
+          <forkCount>${cpu.count}</forkCount>
+          <reuseForks>false</reuseForks>
           <excludes>
             <!-- tests to exclude -->
             <exclude>${testExclusions}</exclude>
diff --git a/spotbugs-exclusions.xml b/spotbugs-exclusions.xml
index 8675964..673bf4b 100644
--- a/spotbugs-exclusions.xml
+++ b/spotbugs-exclusions.xml
@@ -9,11 +9,26 @@
         <Class name="dev.openfeature.sdk.OpenFeatureAPI"/>
         <Bug pattern="MS_EXPOSE_REP"/>
     </And>
-    <!-- similarly, client using the singleton doesn't seem bad -->
+    <!-- evaluation context and hooks are mutable if mutable impl is used -->
+    <And>
+        <Class name="dev.openfeature.sdk.OpenFeatureClient"/>
+        <Bug pattern="EI_EXPOSE_REP"/>
+    </And>
     <And>
         <Class name="dev.openfeature.sdk.OpenFeatureClient"/>
         <Bug pattern="EI_EXPOSE_REP2"/>
     </And>
+    <And>
+        <Class name="dev.openfeature.sdk.OpenFeatureAPI"/>
+        <Bug pattern="EI_EXPOSE_REP"/>
+    </And>
+    <And>
+        <Class name="dev.openfeature.sdk.OpenFeatureAPI"/>
+        <Bug pattern="EI_EXPOSE_REP2"/>
+    </And>
+
+
+
 
     <!-- Test class that should be excluded -->
     <Match>
diff --git a/src/main/java/dev/openfeature/sdk/Client.java b/src/main/java/dev/openfeature/sdk/Client.java
index 07015a6..a4ccf26 100644
--- a/src/main/java/dev/openfeature/sdk/Client.java
+++ b/src/main/java/dev/openfeature/sdk/Client.java
@@ -32,5 +32,5 @@ public interface Client extends Features {
      * Fetch the hooks associated to this client.
      * @return A list of {@link Hook}s.
      */
-    List<Hook> getClientHooks();
+    List<Hook> getHooks();
 }
diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java
index 5918fa0..e85f4e1 100644
--- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java
+++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java
@@ -1,43 +1,41 @@
 package dev.openfeature.sdk;
 
-import lombok.Getter;
-import lombok.Setter;
-
-import javax.annotation.Nullable;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
+import javax.annotation.Nullable;
+
+import dev.openfeature.sdk.internal.AutoCloseableLock;
+import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock;
+
 /**
  * A global singleton which holds base configuration for the OpenFeature library.
  * Configuration here will be shared across all {@link Client}s.
  */
 public class OpenFeatureAPI {
-    private static OpenFeatureAPI api;
-    @Getter
-    @Setter
+    // package-private multi-read/single-write lock
+    static AutoCloseableReentrantReadWriteLock hooksLock = new AutoCloseableReentrantReadWriteLock();
+    static AutoCloseableReentrantReadWriteLock providerLock = new AutoCloseableReentrantReadWriteLock();
+    static AutoCloseableReentrantReadWriteLock contextLock = new AutoCloseableReentrantReadWriteLock();
     private FeatureProvider provider;
-    @Getter
-    @Setter
     private EvaluationContext evaluationContext;
-    @Getter
     private List<Hook> apiHooks;
 
-    public OpenFeatureAPI() {
+    private OpenFeatureAPI() {
         this.apiHooks = new ArrayList<>();
     }
 
+    private static class SingletonHolder {
+        private static final OpenFeatureAPI INSTANCE = new OpenFeatureAPI();
+    }
+
     /**
      * Provisions the {@link OpenFeatureAPI} singleton (if needed) and returns it.
      * @return The singleton instance.
      */
     public static OpenFeatureAPI getInstance() {
-        synchronized (OpenFeatureAPI.class) {
-            if (api == null) {
-                api = new OpenFeatureAPI();
-            }
-        }
-        return api;
+        return SingletonHolder.INSTANCE;
     }
 
     public Metadata getProviderMetadata() {
@@ -56,11 +54,66 @@ public class OpenFeatureAPI {
         return new OpenFeatureClient(this, name, version);
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    public void setEvaluationContext(EvaluationContext evaluationContext) {
+        try (AutoCloseableLock __ = contextLock.writeLockAutoCloseable()) {
+            this.evaluationContext = evaluationContext;
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public EvaluationContext getEvaluationContext() {
+        try (AutoCloseableLock __ = contextLock.readLockAutoCloseable()) {
+            return this.evaluationContext;
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public void setProvider(FeatureProvider provider) {
+        try (AutoCloseableLock __ = providerLock.writeLockAutoCloseable()) {
+            this.provider = provider;
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public FeatureProvider getProvider() {
+        try (AutoCloseableLock __ = providerLock.readLockAutoCloseable()) {
+            return this.provider;
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
     public void addHooks(Hook... hooks) {
-        this.apiHooks.addAll(Arrays.asList(hooks));
+        try (AutoCloseableLock __ = hooksLock.writeLockAutoCloseable()) {
+            this.apiHooks.addAll(Arrays.asList(hooks));
+        }
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    public List<Hook> getHooks() {
+        try (AutoCloseableLock __ = hooksLock.readLockAutoCloseable()) {
+            return this.apiHooks;
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
     public void clearHooks() {
-        this.apiHooks.clear();
+        try (AutoCloseableLock __ = hooksLock.writeLockAutoCloseable()) {
+            this.apiHooks.clear();
+        }
     }
 }
diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java
index 0f99e49..3759ece 100644
--- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java
+++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java
@@ -8,9 +8,10 @@ import java.util.Map;
 
 import dev.openfeature.sdk.exceptions.GeneralError;
 import dev.openfeature.sdk.exceptions.OpenFeatureError;
+import dev.openfeature.sdk.internal.AutoCloseableLock;
+import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock;
 import dev.openfeature.sdk.internal.ObjectUtils;
 import lombok.Getter;
-import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
 
 @Slf4j
@@ -22,12 +23,10 @@ public class OpenFeatureClient implements Client {
     private final String name;
     @Getter
     private final String version;
-    @Getter
     private final List<Hook> clientHooks;
     private final HookSupport hookSupport;
-
-    @Getter
-    @Setter
+    AutoCloseableReentrantReadWriteLock hooksLock = new AutoCloseableReentrantReadWriteLock();
+    AutoCloseableReentrantReadWriteLock contextLock = new AutoCloseableReentrantReadWriteLock();
     private EvaluationContext evaluationContext;
 
     /**
@@ -46,9 +45,44 @@ public class OpenFeatureClient implements Client {
         this.hookSupport = new HookSupport();
     }
 
+    /**
+     * {@inheritDoc}
+     */
     @Override
     public void addHooks(Hook... hooks) {
-        this.clientHooks.addAll(Arrays.asList(hooks));
+        try (AutoCloseableLock __ = this.hooksLock.writeLockAutoCloseable()) {
+            this.clientHooks.addAll(Arrays.asList(hooks));
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public List<Hook> getHooks() {
+        try (AutoCloseableLock __ = this.hooksLock.readLockAutoCloseable()) {
+            return this.clientHooks;
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public void setEvaluationContext(EvaluationContext evaluationContext) {
+        try (AutoCloseableLock __ = contextLock.writeLockAutoCloseable()) {
+            this.evaluationContext = evaluationContext;
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public EvaluationContext getEvaluationContext() {
+        try (AutoCloseableLock __ = contextLock.readLockAutoCloseable()) {
+            return this.evaluationContext;
+        }
     }
 
     private <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key, T defaultValue,
@@ -57,34 +91,41 @@ public class OpenFeatureClient implements Client {
                 () -> FlagEvaluationOptions.builder().build());
         Map<String, Object> hints = Collections.unmodifiableMap(flagOptions.getHookHints());
         ctx = ObjectUtils.defaultIfNull(ctx, () -> new MutableContext());
-        FeatureProvider provider = ObjectUtils.defaultIfNull(openfeatureApi.getProvider(), () -> {
-            log.debug("No provider configured, using no-op provider.");
-            return new NoOpProvider();
-        });
+
 
         FlagEvaluationDetails<T> details = null;
         List<Hook> mergedHooks = null;
         HookContext<T> hookCtx = null;
+        FeatureProvider provider = null;
 
         try {
+            final EvaluationContext apiContext;
+            final EvaluationContext clientContext;
 
-            hookCtx = HookContext.from(key, type, this.getMetadata(),
-                    openfeatureApi.getProvider().getMetadata(), ctx, defaultValue);
+            // openfeatureApi.getProvider() must be called once to maintain a consistent reference
+            provider = ObjectUtils.defaultIfNull(openfeatureApi.getProvider(), () -> {
+                log.debug("No provider configured, using no-op provider.");
+                return new NoOpProvider();
+            });
 
             mergedHooks = ObjectUtils.merge(provider.getProviderHooks(), flagOptions.getHooks(), clientHooks,
-                    openfeatureApi.getApiHooks());
+                    openfeatureApi.getHooks());
 
-            EvaluationContext ctxFromHook = hookSupport.beforeHooks(type, hookCtx, mergedHooks, hints);
-
-            EvaluationContext invocationCtx = ctx.merge(ctxFromHook);
+            hookCtx = HookContext.from(key, type, this.getMetadata(),
+                    provider.getMetadata(), ctx, defaultValue);
 
             // merge of: API.context, client.context, invocation.context
-            EvaluationContext apiContext = openfeatureApi.getEvaluationContext() != null
+            apiContext = openfeatureApi.getEvaluationContext() != null
                     ? openfeatureApi.getEvaluationContext()
                     : new MutableContext();
-            EvaluationContext clientContext = openfeatureApi.getEvaluationContext() != null
+            clientContext = openfeatureApi.getEvaluationContext() != null
                     ? this.getEvaluationContext()
                     : new MutableContext();
+
+            EvaluationContext ctxFromHook = hookSupport.beforeHooks(type, hookCtx, mergedHooks, hints);
+
+            EvaluationContext invocationCtx = ctx.merge(ctxFromHook);
+
             EvaluationContext mergedCtx = apiContext.merge(clientContext.merge(invocationCtx));
 
             ProviderEvaluation<T> providerEval = (ProviderEvaluation<T>) createProviderEvaluation(type, key,
diff --git a/src/main/java/dev/openfeature/sdk/internal/AutoCloseableLock.java b/src/main/java/dev/openfeature/sdk/internal/AutoCloseableLock.java
new file mode 100644
index 0000000..41fb5dc
--- /dev/null
+++ b/src/main/java/dev/openfeature/sdk/internal/AutoCloseableLock.java
@@ -0,0 +1,10 @@
+package dev.openfeature.sdk.internal;
+
+public interface AutoCloseableLock extends AutoCloseable {
+    
+    /**
+     * Override the exception in AutoClosable.
+     */
+    @Override
+    void close();
+}
diff --git a/src/main/java/dev/openfeature/sdk/internal/AutoCloseableReentrantReadWriteLock.java b/src/main/java/dev/openfeature/sdk/internal/AutoCloseableReentrantReadWriteLock.java
new file mode 100644
index 0000000..92827ef
--- /dev/null
+++ b/src/main/java/dev/openfeature/sdk/internal/AutoCloseableReentrantReadWriteLock.java
@@ -0,0 +1,28 @@
+package dev.openfeature.sdk.internal;
+
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+/**
+ * A utility class that wraps a multi-read/single-write lock construct as AutoCloseable, so it can
+ * be used in a try-with-resources.
+ */
+public class AutoCloseableReentrantReadWriteLock extends ReentrantReadWriteLock {
+
+    /**
+     * Get the single write lock as an AutoCloseableLock.
+     * @return unlock method ref
+     */
+    public AutoCloseableLock writeLockAutoCloseable() {
+        this.writeLock().lock();
+        return this.writeLock()::unlock;
+    }
+
+    /**
+     * Get the multi read lock as an AutoCloseableLock.
+     * @return unlock method ref
+     */
+    public AutoCloseableLock readLockAutoCloseable() {
+        this.readLock().lock();
+        return this.readLock()::unlock;
+    }
+}
\ No newline at end of file


Test patch:
diff --git a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java
index 454f167..12e03ab 100644
--- a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java
+++ b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java
@@ -6,12 +6,14 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
+import java.util.Arrays;
+import java.util.Map;
+import java.util.Optional;
+
 import org.junit.jupiter.api.Test;
 
 import dev.openfeature.sdk.fixtures.HookFixtures;
 
-import java.util.Arrays;
-
 class DeveloperExperienceTest implements HookFixtures {
     transient String flagKey = "mykey";
 
@@ -90,4 +92,34 @@ class DeveloperExperienceTest implements HookFixtures {
         assertEquals(Reason.ERROR.toString(), retval.getReason());
         assertFalse(retval.getValue());
     }
+
+    @Test
+    void providerLockedPerTransaction() throws InterruptedException {
+
+        class MutatingHook implements Hook {
+
+            @Override
+            // change the provider during a before hook - this should not impact the evaluation in progress
+            public Optional before(HookContext ctx, Map hints) {
+                OpenFeatureAPI.getInstance().setProvider(new NoOpProvider());
+                return Optional.empty();
+            }
+        }
+        
+        final String defaultValue = "string-value";
+        final OpenFeatureAPI api = OpenFeatureAPI.getInstance();
+        final Client client = api.getClient();
+        api.setProvider(new DoSomethingProvider());
+        api.addHooks(new MutatingHook());
+
+        // if provider is changed during an evaluation transaction it should proceed with the original provider
+        String doSomethingValue = client.getStringValue("val", defaultValue);
+        assertEquals(new StringBuilder(defaultValue).reverse().toString(), doSomethingValue);
+
+        api.clearHooks();
+        
+        // subsequent evaluations should now use new provider set by hook
+        String noOpValue = client.getStringValue("val", defaultValue);
+        assertEquals(noOpValue, defaultValue);
+    }
 }
diff --git a/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java b/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java
index 37fd20f..d87fa37 100644
--- a/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java
+++ b/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java
@@ -2,6 +2,7 @@ package dev.openfeature.sdk;
 
 public class DoSomethingProvider implements FeatureProvider {
 
+    public static final String name = "Something";
     private EvaluationContext savedContext;
 
     public EvaluationContext getMergedContext() {
@@ -10,7 +11,7 @@ public class DoSomethingProvider implements FeatureProvider {
 
     @Override
     public Metadata getMetadata() {
-        return () -> "test";
+        return () -> name;
     }
 
     @Override
diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java
index 75b6e5b..0bf9a6d 100644
--- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java
+++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java
@@ -53,7 +53,7 @@ class FlagEvaluationSpecTest implements HookFixtures {
     @Test void provider_metadata() {
         OpenFeatureAPI api = OpenFeatureAPI.getInstance();
         api.setProvider(new DoSomethingProvider());
-        assertEquals("test", api.getProviderMetadata().getName());
+        assertEquals(DoSomethingProvider.name, api.getProviderMetadata().getName());
     }
 
     @Specification(number="1.1.3", text="The API MUST provide a function to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed.")
@@ -63,12 +63,12 @@ class FlagEvaluationSpecTest implements HookFixtures {
         OpenFeatureAPI api = OpenFeatureAPI.getInstance();
         api.addHooks(h1);
 
-        assertEquals(1, api.getApiHooks().size());
-        assertEquals(h1, api.getApiHooks().get(0));
+        assertEquals(1, api.getHooks().size());
+        assertEquals(h1, api.getHooks().get(0));
 
         api.addHooks(h2);
-        assertEquals(2, api.getApiHooks().size());
-        assertEquals(h2, api.getApiHooks().get(1));
+        assertEquals(2, api.getHooks().size());
+        assertEquals(h2, api.getHooks().get(1));
     }
 
     @Specification(number="1.1.5", text="The API MUST provide a function for creating a client which accepts the following options:  - name (optional): A logical string identifier for the client.")
@@ -85,7 +85,7 @@ class FlagEvaluationSpecTest implements HookFixtures {
         Hook m2 = mock(Hook.class);
         c.addHooks(m1);
         c.addHooks(m2);
-        List<Hook> hooks = c.getClientHooks();
+        List<Hook> hooks = c.getHooks();
         assertEquals(2, hooks.size());
         assertTrue(hooks.contains(m1));
         assertTrue(hooks.contains(m2));
diff --git a/src/test/java/dev/openfeature/sdk/LockingTest.java b/src/test/java/dev/openfeature/sdk/LockingTest.java
new file mode 100644
index 0000000..1a8dac9
--- /dev/null
+++ b/src/test/java/dev/openfeature/sdk/LockingTest.java
@@ -0,0 +1,132 @@
+package dev.openfeature.sdk;
+
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock;
+
+class LockingTest {
+
+    private static OpenFeatureAPI api;
+    private OpenFeatureClient client;
+    private AutoCloseableReentrantReadWriteLock apiContextLock;
+    private AutoCloseableReentrantReadWriteLock apiHooksLock;
+    private AutoCloseableReentrantReadWriteLock apiProviderLock;
+    private AutoCloseableReentrantReadWriteLock clientContextLock;
+    private AutoCloseableReentrantReadWriteLock clientHooksLock;
+    
+    @BeforeAll
+    static void beforeAll() {
+        api = OpenFeatureAPI.getInstance();
+    }
+
+    @BeforeEach
+    void beforeEach() {
+        client = (OpenFeatureClient) api.getClient();
+        
+        apiContextLock = setupLock(apiContextLock, mockInnerReadLock(), mockInnerWriteLock());
+        apiProviderLock = setupLock(apiProviderLock, mockInnerReadLock(), mockInnerWriteLock());
+        apiHooksLock = setupLock(apiHooksLock, mockInnerReadLock(), mockInnerWriteLock());
+        OpenFeatureAPI.contextLock = apiContextLock;
+        OpenFeatureAPI.providerLock = apiProviderLock;
+        OpenFeatureAPI.hooksLock = apiHooksLock;
+
+        clientContextLock = setupLock(clientContextLock, mockInnerReadLock(), mockInnerWriteLock());
+        clientHooksLock = setupLock(clientHooksLock, mockInnerReadLock(), mockInnerWriteLock());
+        client.contextLock = clientContextLock;
+        client.hooksLock = clientHooksLock;
+    }
+
+    @Test
+    void addHooksShouldWriteLockAndUnlock() {
+        client.addHooks(new Hook() {
+        });
+        verify(clientHooksLock.writeLock()).lock();
+        verify(clientHooksLock.writeLock()).unlock();
+
+        api.addHooks(new Hook() {
+        });
+        verify(apiHooksLock.writeLock()).lock();
+        verify(apiHooksLock.writeLock()).unlock();
+    }
+
+    @Test
+    void getHooksShouldReadLockAndUnlock() {
+        client.getHooks();
+        verify(clientHooksLock.readLock()).lock();
+        verify(clientHooksLock.readLock()).unlock();
+
+        api.getHooks();
+        verify(apiHooksLock.readLock()).lock();
+        verify(apiHooksLock.readLock()).unlock();
+    }
+
+    @Test
+    void setContextShouldWriteLockAndUnlock() {
+        client.setEvaluationContext(new MutableContext());
+        verify(clientContextLock.writeLock()).lock();
+        verify(clientContextLock.writeLock()).unlock();
+
+        api.setEvaluationContext(new MutableContext());
+        verify(apiContextLock.writeLock()).lock();
+        verify(apiContextLock.writeLock()).unlock();
+    }
+
+    @Test
+    void getContextShouldReadLockAndUnlock() {
+        client.getEvaluationContext();
+        verify(clientContextLock.readLock()).lock();
+        verify(clientContextLock.readLock()).unlock();
+
+        api.getEvaluationContext();
+        verify(apiContextLock.readLock()).lock();
+        verify(apiContextLock.readLock()).unlock();
+    }
+
+    @Test
+    void setProviderShouldWriteLockAndUnlock() {
+        api.setProvider(new DoSomethingProvider());
+        verify(apiProviderLock.writeLock()).lock();
+        verify(apiProviderLock.writeLock()).unlock();
+    }
+
+    @Test
+    void clearHooksShouldWriteLockAndUnlock() {
+        api.clearHooks();
+        verify(apiHooksLock.writeLock()).lock();
+        verify(apiHooksLock.writeLock()).unlock();
+    }
+
+    private static ReentrantReadWriteLock.ReadLock mockInnerReadLock() {
+        ReentrantReadWriteLock.ReadLock readLockMock = mock(ReentrantReadWriteLock.ReadLock.class);
+        doNothing().when(readLockMock).lock();
+        doNothing().when(readLockMock).unlock();
+        return readLockMock;
+    }
+
+    private static ReentrantReadWriteLock.WriteLock mockInnerWriteLock() {
+        ReentrantReadWriteLock.WriteLock writeLockMock = mock(ReentrantReadWriteLock.WriteLock.class);
+        doNothing().when(writeLockMock).lock();
+        doNothing().when(writeLockMock).unlock();
+        return writeLockMock;
+    }
+
+    private AutoCloseableReentrantReadWriteLock setupLock(AutoCloseableReentrantReadWriteLock lock,
+            AutoCloseableReentrantReadWriteLock.ReadLock readlock,
+            AutoCloseableReentrantReadWriteLock.WriteLock writeLock) {
+        lock = mock(AutoCloseableReentrantReadWriteLock.class);
+        when(lock.readLockAutoCloseable()).thenCallRealMethod();
+        when(lock.readLock()).thenReturn(readlock);
+        when(lock.writeLockAutoCloseable()).thenCallRealMethod();
+        when(lock.writeLock()).thenReturn(writeLock);
+        return lock;
+    }
+}
\ No newline at end of file
diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java
index 14446c1..eab962a 100644
--- a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java
+++ b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java
@@ -20,7 +20,7 @@ class OpenFeatureClientTest implements HookFixtures {
         TEST_LOGGER.clear();
         OpenFeatureAPI api = mock(OpenFeatureAPI.class);
         when(api.getProvider()).thenReturn(new DoSomethingProvider());
-        when(api.getApiHooks()).thenReturn(Arrays.asList(mockBooleanHook(), mockStringHook()));
+        when(api.getHooks()).thenReturn(Arrays.asList(mockBooleanHook(), mockStringHook()));
 
         OpenFeatureClient client = new OpenFeatureClient(api, "name", "version");