sashapolo commented on code in PR #3189:
URL: https://github.com/apache/ignite-3/pull/3189#discussion_r1483982465


##########
modules/vault/src/testFixtures/java/org/apache/ignite/internal/vault/inmemory/InMemoryVaultService.java:
##########
@@ -37,85 +36,58 @@
  */
 public class InMemoryVaultService implements VaultService {
     /** Map to store values. */
-    private final NavigableMap<ByteArray, byte[]> storage = new TreeMap<>();
-
-    /** Mutex. */
-    private final Object mux = new Object();
+    private final NavigableMap<ByteArray, byte[]> storage = 
synchronizedNavigableMap(new TreeMap<>());
 
     @Override
     public void start() {
         // No-op.
     }
 
-    /** {@inheritDoc} */
     @Override
     public void close() {
         // No-op.
     }
 
-    /** {@inheritDoc} */
     @Override
-    public CompletableFuture<VaultEntry> get(ByteArray key) {
-        return supplyAsync(() -> {
-            synchronized (mux) {
-                byte[] value = storage.get(key);
+    public VaultEntry get(ByteArray key) {
+        byte[] value = storage.get(key);
 
-                return value == null ? null : new VaultEntry(key, 
storage.get(key));
-            }
-        });
+        return value == null ? null : new VaultEntry(key, value);
     }
 
-    /** {@inheritDoc} */
     @Override
-    public CompletableFuture<Void> put(ByteArray key, byte @Nullable [] val) {
-        return runAsync(() -> {
-            synchronized (mux) {
-                storage.put(key, val);
-            }
-        });
+    public void put(ByteArray key, byte @Nullable [] val) {
+        storage.put(key, val);
     }
 
-    /** {@inheritDoc} */
     @Override
-    public CompletableFuture<Void> remove(ByteArray key) {
-        return runAsync(() -> {
-            synchronized (mux) {
-                storage.remove(key);
-            }
-        });
+    public void remove(ByteArray key) {
+        storage.remove(key);
     }
 
-    /** {@inheritDoc} */
     @Override
     public Cursor<VaultEntry> range(ByteArray fromKey, ByteArray toKey) {
-        Iterator<VaultEntry> it;
-
         if (fromKey.compareTo(toKey) >= 0) {
-            it = Collections.emptyIterator();
-        } else {
-            synchronized (mux) {
-                it = storage.subMap(fromKey, toKey).entrySet().stream()
-                        .map(e -> new VaultEntry(new ByteArray(e.getKey()), 
e.getValue()))
-                        .iterator();
-            }
+            return CursorUtils.emptyCursor();
         }
 
-        return Cursor.fromBareIterator(it);
+        synchronized (storage) {

Review Comment:
   This is strange, here's an excerpt from its javadoc:
   ```
        * It is imperative that the user manually synchronize on the returned
        * navigable map when traversing any of its collection views, or the
        * collections views of any of its {@code subMap}, {@code headMap} or
        * {@code tailMap} views, via {@link Iterator}, {@link Spliterator} or
        * {@link Stream}:
        * <pre>
        *  NavigableMap m = Collections.synchronizedNavigableMap(new TreeMap());
        *      ...
        *  Set s = m.keySet();  // Needn't be in synchronized block
        *      ...
        *  synchronized (m) {  // Synchronizing on m, not s!
        *      Iterator i = s.iterator(); // Must be in synchronized block
        *      while (i.hasNext())
        *          foo(i.next());
        *  }
        * </pre>  
   ```



-- 
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]

Reply via email to