michael-o commented on a change in pull request #77:
URL: https://github.com/apache/maven-resolver/pull/77#discussion_r549341209
##########
File path: maven-resolver-impl/pom.xml
##########
@@ -49,6 +49,10 @@
<groupId>org.apache.maven.resolver</groupId>
<artifactId>maven-resolver-spi</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.maven.resolver</groupId>
+ <artifactId>maven-resolver-named</artifactId>
Review comment:
I think this is a bad name for an artifact id. It says that the Resolver
is named. `maven-resolver-synccontext-named`?
##########
File path:
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java
##########
@@ -0,0 +1,91 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.named.providers.LocalReadWriteLockProvider;
+import org.eclipse.aether.spi.synccontext.SyncContextFactory;
+
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Named {@link SyncContextFactory} implementation that selects underlying
{@link NamedLockFactory} implementation
+ * at creation.
+ */
+@Named
+@Singleton
+public final class NamedSyncContextFactory
+ implements SyncContextFactory
+{
+ private static final long TIME = Long.getLong(
Review comment:
Isn't this rather a timeout?
##########
File path:
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link
SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+ private static final String DEFAULT_DISCRIMINATOR_DIGEST =
"da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+ private static final String DEFAULT_HOSTNAME = "localhost";
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(
SyncContextFactoryAdapter.class );
+
+ private final NamedLockFactory namedLockFactory;
+
+ private final long time;
+
+ private final TimeUnit timeUnit;
+
+ private final String hostname;
+
+ public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+ final long time,
+ final TimeUnit timeUnit )
+ {
+ this.namedLockFactory = namedLockFactory;
+ this.time = time;
+ this.timeUnit = timeUnit;
+ this.hostname = getHostname();
+ }
+
+ private String getHostname()
+ {
+ try
+ {
+ return InetAddress.getLocalHost().getHostName();
+ }
+ catch ( UnknownHostException e )
+ {
+ LOGGER.warn( "Failed to get hostname, using '{}'",
DEFAULT_HOSTNAME, e );
+ return DEFAULT_HOSTNAME;
+ }
+ }
+
+ public SyncContext newInstance( final RepositorySystemSession session,
final boolean shared )
+ {
+ return new AdaptedLockSyncContext( session, hostname,
namedLockFactory, time, timeUnit, shared );
+ }
+
+ public void shutdown()
+ {
+ namedLockFactory.shutdown();
+ }
+
+ private static class AdaptedLockSyncContext
+ implements SyncContext
+ {
+ private static final String CONFIG_PROP_DISCRIMINATOR =
"aether.syncContext.named.discriminator";
+
+ private static final String KEY_PREFIX = "mvn:resolver:";
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(
AdaptedLockSyncContext.class );
+
+ private final RepositorySystemSession session;
+
+ private final String hostname;
+
+ private final NamedLockFactory namedLockFactory;
+
+ private final long time;
+
+ private final TimeUnit timeUnit;
+
+ private final boolean shared;
+
+ private final ArrayDeque<NamedLock> locks;
+
+ private AdaptedLockSyncContext( final RepositorySystemSession session,
+ final String hostname,
+ final NamedLockFactory
namedLockFactory,
+ final long time,
+ final TimeUnit timeUnit,
+ final boolean shared )
+ {
+ this.session = session;
+ this.hostname = hostname;
+ this.namedLockFactory = namedLockFactory;
+ this.time = time;
+ this.timeUnit = timeUnit;
+ this.shared = shared;
+ this.locks = new ArrayDeque<>();
+ }
+
+ @Override
+ public void acquire( Collection<? extends Artifact> artifacts,
+ Collection<? extends Metadata> metadatas )
+ {
+ // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+ // We must acquire multiple locks always in the same order!
+ Collection<String> keys = new TreeSet<>();
+ if ( artifacts != null )
+ {
+ for ( Artifact artifact : artifacts )
+ {
+ // TODO Should we include extension and classifier too?
+ String key = "artifact:" + artifact.getGroupId() + ":"
+ + artifact.getArtifactId() + ":" +
artifact.getBaseVersion();
+ keys.add( key );
+ }
+ }
+
+ if ( metadatas != null )
+ {
+ for ( Metadata metadata : metadatas )
+ {
+ StringBuilder key = new StringBuilder( "metadata:" );
+ if ( !metadata.getGroupId().isEmpty() )
+ {
+ key.append( metadata.getGroupId() );
+ if ( !metadata.getArtifactId().isEmpty() )
+ {
+ key.append( ':' ).append( metadata.getArtifactId()
);
+ if ( !metadata.getVersion().isEmpty() )
+ {
+ key.append( ':' ).append(
metadata.getVersion() );
+ }
+ }
+ }
+ keys.add( key.toString() );
+ }
+ }
+
+ if ( keys.isEmpty() )
+ {
+ return;
+ }
+
+ String discriminator = createDiscriminator();
+ LOGGER.trace( "Using key discriminator '{}' during this session",
discriminator );
+
+ LOGGER.trace( "Need {} {} lock(s) for {}", keys.size(), shared ?
"read" : "write", keys );
+ int acquiredLockCount = 0;
+ for ( String key : keys )
+ {
+ NamedLock namedLock = namedLockFactory.getLock( KEY_PREFIX +
discriminator + ":" + key );
+ try
+ {
+ boolean locked;
+ if ( shared )
+ {
+ locked = namedLock.lockShared( time, timeUnit );
+ }
+ else
+ {
+ locked = namedLock.lockExclusively( time, timeUnit );
+ }
+
+ if ( !locked )
+ {
+ namedLock.close();
+ throw new IllegalStateException( "Could not lock "
+ + namedLock.name() + " (shared=" + shared +
")" );
+ }
+
+ locks.push( namedLock );
+ acquiredLockCount++;
+ }
+ catch ( InterruptedException e )
+ {
+ Thread.currentThread().interrupt();
+ throw new RuntimeException( e );
+ }
+ }
+ LOGGER.trace( "Total new locks acquired: {}", acquiredLockCount );
+ }
+
+ private String createDiscriminator()
+ {
+ String discriminator = ConfigUtils.getString( session, null,
CONFIG_PROP_DISCRIMINATOR );
+
+ if ( discriminator == null || discriminator.isEmpty() )
+ {
+ File basedir = session.getLocalRepository().getBasedir();
+ discriminator = hostname + ":" + basedir;
+ try
+ {
+ Map<String, Object> checksums = ChecksumUtils.calc(
+ discriminator.getBytes( StandardCharsets.UTF_8 ),
+ Collections.singletonList( "SHA-1" ) );
+ Object checksum = checksums.get( "SHA-1" );
+
+ if ( checksum instanceof Exception )
+ {
+ throw ( Exception ) checksum;
+ }
+
+ return String.valueOf( checksum );
+ }
+ catch ( Exception e )
+ {
+ LOGGER.warn( "Failed to calculate discriminator digest,
using '{}'",
+ DEFAULT_DISCRIMINATOR_DIGEST, e );
+ return DEFAULT_DISCRIMINATOR_DIGEST;
+ }
+ }
+
+ return discriminator;
+ }
+
+ @Override
+ public void close()
+ {
+ if ( locks.isEmpty() )
+ {
+ return;
+ }
+
+ // Release locks in reverse insertion order
+ int released = 0;
+ while ( !locks.isEmpty() )
Review comment:
I understand that the array deque gives you the pop for free, but the
map has more benefits.
##########
File path:
maven-resolver-named-hazelcast/src/test/resources/hazelcast-client.xml
##########
@@ -0,0 +1,39 @@
+<?xml version="1.0"?>
+
+<!--
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+-->
+
+<hazelcast-client xmlns="http://www.hazelcast.com/schema/client-config";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+
xsi:schemaLocation="http://www.hazelcast.com/schema/client-config
+
http://www.hazelcast.com/schema/client-config/hazelcast-client-config-3.12.xsd";>
+
+ <group>
+ <name>maven-resolved-named</name>
+ </group>
+ <instance-name>client</instance-name>
+ <properties>
+ <property name="hazelcast.logging.type">slf4j</property>
+ </properties>
+ <network>
+ <cluster-members>
+ <address>127.0.0.1</address>
Review comment:
This shoulc rather be `localhost` host otherwise other IPv4 local
address won't bind or `::1`.
##########
File path:
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/NamedSyncContextFactory.java
##########
@@ -0,0 +1,91 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.named.providers.LocalReadWriteLockProvider;
+import org.eclipse.aether.spi.synccontext.SyncContextFactory;
+
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Named {@link SyncContextFactory} implementation that selects underlying
{@link NamedLockFactory} implementation
+ * at creation.
+ */
+@Named
+@Singleton
+public final class NamedSyncContextFactory
Review comment:
All system properties for this class should start with
`aether.syncContext.named.` to scope them for Resolver property.
##########
File path:
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link
SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+ private static final String DEFAULT_DISCRIMINATOR_DIGEST =
"da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+ private static final String DEFAULT_HOSTNAME = "localhost";
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(
SyncContextFactoryAdapter.class );
+
+ private final NamedLockFactory namedLockFactory;
+
+ private final long time;
+
+ private final TimeUnit timeUnit;
+
+ private final String hostname;
+
+ public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+ final long time,
+ final TimeUnit timeUnit )
+ {
+ this.namedLockFactory = namedLockFactory;
+ this.time = time;
+ this.timeUnit = timeUnit;
+ this.hostname = getHostname();
+ }
+
+ private String getHostname()
+ {
+ try
+ {
+ return InetAddress.getLocalHost().getHostName();
+ }
+ catch ( UnknownHostException e )
+ {
+ LOGGER.warn( "Failed to get hostname, using '{}'",
DEFAULT_HOSTNAME, e );
+ return DEFAULT_HOSTNAME;
+ }
+ }
+
+ public SyncContext newInstance( final RepositorySystemSession session,
final boolean shared )
+ {
+ return new AdaptedLockSyncContext( session, hostname,
namedLockFactory, time, timeUnit, shared );
+ }
+
+ public void shutdown()
+ {
+ namedLockFactory.shutdown();
+ }
+
+ private static class AdaptedLockSyncContext
+ implements SyncContext
+ {
+ private static final String CONFIG_PROP_DISCRIMINATOR =
"aether.syncContext.named.discriminator";
+
+ private static final String KEY_PREFIX = "mvn:resolver:";
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(
AdaptedLockSyncContext.class );
+
+ private final RepositorySystemSession session;
+
+ private final String hostname;
+
+ private final NamedLockFactory namedLockFactory;
+
+ private final long time;
+
+ private final TimeUnit timeUnit;
+
+ private final boolean shared;
+
+ private final ArrayDeque<NamedLock> locks;
+
+ private AdaptedLockSyncContext( final RepositorySystemSession session,
+ final String hostname,
+ final NamedLockFactory
namedLockFactory,
+ final long time,
+ final TimeUnit timeUnit,
+ final boolean shared )
+ {
+ this.session = session;
+ this.hostname = hostname;
+ this.namedLockFactory = namedLockFactory;
+ this.time = time;
+ this.timeUnit = timeUnit;
+ this.shared = shared;
+ this.locks = new ArrayDeque<>();
+ }
+
+ @Override
+ public void acquire( Collection<? extends Artifact> artifacts,
+ Collection<? extends Metadata> metadatas )
+ {
+ // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+ // We must acquire multiple locks always in the same order!
+ Collection<String> keys = new TreeSet<>();
+ if ( artifacts != null )
+ {
+ for ( Artifact artifact : artifacts )
+ {
+ // TODO Should we include extension and classifier too?
+ String key = "artifact:" + artifact.getGroupId() + ":"
+ + artifact.getArtifactId() + ":" +
artifact.getBaseVersion();
+ keys.add( key );
+ }
+ }
+
+ if ( metadatas != null )
+ {
+ for ( Metadata metadata : metadatas )
+ {
+ StringBuilder key = new StringBuilder( "metadata:" );
+ if ( !metadata.getGroupId().isEmpty() )
+ {
+ key.append( metadata.getGroupId() );
+ if ( !metadata.getArtifactId().isEmpty() )
+ {
+ key.append( ':' ).append( metadata.getArtifactId()
);
+ if ( !metadata.getVersion().isEmpty() )
+ {
+ key.append( ':' ).append(
metadata.getVersion() );
+ }
+ }
+ }
+ keys.add( key.toString() );
+ }
+ }
+
+ if ( keys.isEmpty() )
+ {
+ return;
+ }
+
+ String discriminator = createDiscriminator();
+ LOGGER.trace( "Using key discriminator '{}' during this session",
discriminator );
+
+ LOGGER.trace( "Need {} {} lock(s) for {}", keys.size(), shared ?
"read" : "write", keys );
+ int acquiredLockCount = 0;
+ for ( String key : keys )
+ {
+ NamedLock namedLock = namedLockFactory.getLock( KEY_PREFIX +
discriminator + ":" + key );
+ try
+ {
+ boolean locked;
+ if ( shared )
+ {
+ locked = namedLock.lockShared( time, timeUnit );
Review comment:
Now this is really a timeout, not just time. I am quite certain that
this will fail with 10 s when large artifacts are transferred. But OK, users
should adapt or we bump to 30 s.
##########
File path:
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link
SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+ private static final String DEFAULT_DISCRIMINATOR_DIGEST =
"da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+ private static final String DEFAULT_HOSTNAME = "localhost";
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(
SyncContextFactoryAdapter.class );
+
+ private final NamedLockFactory namedLockFactory;
+
+ private final long time;
+
+ private final TimeUnit timeUnit;
+
+ private final String hostname;
+
+ public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+ final long time,
+ final TimeUnit timeUnit )
+ {
+ this.namedLockFactory = namedLockFactory;
+ this.time = time;
+ this.timeUnit = timeUnit;
+ this.hostname = getHostname();
+ }
+
+ private String getHostname()
+ {
+ try
+ {
+ return InetAddress.getLocalHost().getHostName();
+ }
+ catch ( UnknownHostException e )
+ {
+ LOGGER.warn( "Failed to get hostname, using '{}'",
DEFAULT_HOSTNAME, e );
+ return DEFAULT_HOSTNAME;
+ }
+ }
+
+ public SyncContext newInstance( final RepositorySystemSession session,
final boolean shared )
+ {
+ return new AdaptedLockSyncContext( session, hostname,
namedLockFactory, time, timeUnit, shared );
+ }
+
+ public void shutdown()
+ {
+ namedLockFactory.shutdown();
+ }
+
+ private static class AdaptedLockSyncContext
+ implements SyncContext
+ {
+ private static final String CONFIG_PROP_DISCRIMINATOR =
"aether.syncContext.named.discriminator";
+
+ private static final String KEY_PREFIX = "mvn:resolver:";
Review comment:
This might be a matter of taste, but I rather prefix the full name here
`maven:resolver:`. See
https://github.com/apache/maven/pull/419/files#diff-c9096a367ccf68804df8750db73e4a5cc8493bfb3d46242442cd7418a9e110e0R248.
##########
File path:
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link
SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+ private static final String DEFAULT_DISCRIMINATOR_DIGEST =
"da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+ private static final String DEFAULT_HOSTNAME = "localhost";
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(
SyncContextFactoryAdapter.class );
+
+ private final NamedLockFactory namedLockFactory;
+
+ private final long time;
+
+ private final TimeUnit timeUnit;
+
+ private final String hostname;
+
+ public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+ final long time,
+ final TimeUnit timeUnit )
+ {
+ this.namedLockFactory = namedLockFactory;
+ this.time = time;
+ this.timeUnit = timeUnit;
+ this.hostname = getHostname();
+ }
+
+ private String getHostname()
+ {
+ try
+ {
+ return InetAddress.getLocalHost().getHostName();
+ }
+ catch ( UnknownHostException e )
+ {
+ LOGGER.warn( "Failed to get hostname, using '{}'",
DEFAULT_HOSTNAME, e );
+ return DEFAULT_HOSTNAME;
+ }
+ }
+
+ public SyncContext newInstance( final RepositorySystemSession session,
final boolean shared )
+ {
+ return new AdaptedLockSyncContext( session, hostname,
namedLockFactory, time, timeUnit, shared );
+ }
+
+ public void shutdown()
+ {
+ namedLockFactory.shutdown();
+ }
+
+ private static class AdaptedLockSyncContext
+ implements SyncContext
+ {
+ private static final String CONFIG_PROP_DISCRIMINATOR =
"aether.syncContext.named.discriminator";
+
+ private static final String KEY_PREFIX = "mvn:resolver:";
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(
AdaptedLockSyncContext.class );
+
+ private final RepositorySystemSession session;
+
+ private final String hostname;
+
+ private final NamedLockFactory namedLockFactory;
+
+ private final long time;
+
+ private final TimeUnit timeUnit;
+
+ private final boolean shared;
+
+ private final ArrayDeque<NamedLock> locks;
+
+ private AdaptedLockSyncContext( final RepositorySystemSession session,
+ final String hostname,
+ final NamedLockFactory
namedLockFactory,
+ final long time,
+ final TimeUnit timeUnit,
+ final boolean shared )
+ {
+ this.session = session;
+ this.hostname = hostname;
+ this.namedLockFactory = namedLockFactory;
+ this.time = time;
+ this.timeUnit = timeUnit;
+ this.shared = shared;
+ this.locks = new ArrayDeque<>();
+ }
+
+ @Override
+ public void acquire( Collection<? extends Artifact> artifacts,
+ Collection<? extends Metadata> metadatas )
+ {
+ // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+ // We must acquire multiple locks always in the same order!
+ Collection<String> keys = new TreeSet<>();
+ if ( artifacts != null )
+ {
+ for ( Artifact artifact : artifacts )
+ {
+ // TODO Should we include extension and classifier too?
+ String key = "artifact:" + artifact.getGroupId() + ":"
+ + artifact.getArtifactId() + ":" +
artifact.getBaseVersion();
+ keys.add( key );
+ }
+ }
+
+ if ( metadatas != null )
+ {
+ for ( Metadata metadata : metadatas )
+ {
+ StringBuilder key = new StringBuilder( "metadata:" );
+ if ( !metadata.getGroupId().isEmpty() )
+ {
+ key.append( metadata.getGroupId() );
+ if ( !metadata.getArtifactId().isEmpty() )
+ {
+ key.append( ':' ).append( metadata.getArtifactId()
);
+ if ( !metadata.getVersion().isEmpty() )
+ {
+ key.append( ':' ).append(
metadata.getVersion() );
+ }
+ }
+ }
+ keys.add( key.toString() );
+ }
+ }
+
+ if ( keys.isEmpty() )
+ {
+ return;
+ }
+
+ String discriminator = createDiscriminator();
+ LOGGER.trace( "Using key discriminator '{}' during this session",
discriminator );
+
+ LOGGER.trace( "Need {} {} lock(s) for {}", keys.size(), shared ?
"read" : "write", keys );
+ int acquiredLockCount = 0;
+ for ( String key : keys )
+ {
+ NamedLock namedLock = namedLockFactory.getLock( KEY_PREFIX +
discriminator + ":" + key );
+ try
+ {
+ boolean locked;
+ if ( shared )
+ {
+ locked = namedLock.lockShared( time, timeUnit );
+ }
+ else
+ {
+ locked = namedLock.lockExclusively( time, timeUnit );
+ }
+
+ if ( !locked )
+ {
+ namedLock.close();
+ throw new IllegalStateException( "Could not lock "
+ + namedLock.name() + " (shared=" + shared +
")" );
+ }
+
+ locks.push( namedLock );
Review comment:
Since you already cache locks, why not a the linked hash map to reuse
the from for reacquision?
##########
File path: maven-resolver-named-hazelcast/src/site/site.xml
##########
@@ -22,7 +22,7 @@ under the License.
<project xmlns="http://maven.apache.org/DECORATION/1.0.0";
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
xsi:schemaLocation="http://maven.apache.org/DECORATION/1.0.0
http://maven.apache.org/xsd/decoration-1.0.0.xsd";
- name="GlobalSyncContext">
+ name="NamedSyncContext">
Review comment:
The name says `NamedSyncContext` although this is Hazelcast?
##########
File path: maven-resolver-named-redisson/pom.xml
##########
@@ -28,51 +28,57 @@
<version>1.7.0-SNAPSHOT</version>
</parent>
- <artifactId>maven-resolver-synccontext-redisson</artifactId>
+ <artifactId>maven-resolver-named-redisson</artifactId>
Review comment:
See comment for `maven-resolver-named`.
##########
File path:
maven-resolver-named/src/main/java/org/eclipse/aether/named/NamedLock.java
##########
@@ -0,0 +1,71 @@
+package org.eclipse.aether.named;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.concurrent.TimeUnit;
+
+/**
+ * A named lock, functionally similar to existing JVM and other
implementations. Does support boxing, but no
+ * lock upgrade is supported. Usual pattern to use this lock:
+ * <pre>
+ * try (NamedLock lock = factory.getLock("resourceName")) {
+ * if (lock.lockExclusively(10L, Timeunit.SECONDS)) {
+ * try {
+ * ... exclusive access to "resourceName" resource gained here
+ * }
+ * finally {
+ * lock.unlock();
+ * }
+ * }
+ * else {
+ * ... failed to gain access within specified time, handle it
+ * }
+ * }
+ * </pre>
+ */
+public interface NamedLock
Review comment:
The documentation should clearly state that every implementation must
support reentrancy. This is required by the `SyncContextFactory`.
##########
File path: maven-resolver-named-redisson/src/site/markdown/index.md.vm
##########
@@ -0,0 +1,69 @@
+${esc.hash} Named Sync Context for Maven Resolver
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+The Named Sync Context Factory is a distributed lock factory for Maven Resolver
+on top of JVM and Hazelcast to provide a fast, concurrent-safe access from one
or multiple Maven instances to the
+same local Maven repository.
+
+For further details about the factory read the
[Javadoc](./apidocs/org/eclipse/aether/synccontext/NamedSyncContextFactory.html).
+
+${esc.hash}${esc.hash} Open Issues/Notes
+
+- It only works when dependency injection is used and not the bundled
`AetherModule` or
+ `ServiceLocator` (Maven uses dependency injection).
+- It includes a lot of trace logging which partially will go way as soon as it
has been stabilized.
+- Usage from plugins has not been tested yet.
+- The `furnace-maven-plugin` does not work this implementation because it uses
`ServiceLocator` instead
+ of dependency injection.
+
+${esc.hash}${esc.hash} Installation/Testing
+
+- Create the directory `${maven.home}/lib/ext/hazelcast/`.
Review comment:
The file path says redisson while this covers Hazelcast.
##########
File path:
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link
SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+ private static final String DEFAULT_DISCRIMINATOR_DIGEST =
"da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+ private static final String DEFAULT_HOSTNAME = "localhost";
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(
SyncContextFactoryAdapter.class );
+
+ private final NamedLockFactory namedLockFactory;
+
+ private final long time;
+
+ private final TimeUnit timeUnit;
+
+ private final String hostname;
+
+ public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+ final long time,
+ final TimeUnit timeUnit )
+ {
+ this.namedLockFactory = namedLockFactory;
+ this.time = time;
+ this.timeUnit = timeUnit;
+ this.hostname = getHostname();
+ }
+
+ private String getHostname()
+ {
+ try
+ {
+ return InetAddress.getLocalHost().getHostName();
+ }
+ catch ( UnknownHostException e )
+ {
+ LOGGER.warn( "Failed to get hostname, using '{}'",
DEFAULT_HOSTNAME, e );
+ return DEFAULT_HOSTNAME;
+ }
+ }
+
+ public SyncContext newInstance( final RepositorySystemSession session,
final boolean shared )
+ {
+ return new AdaptedLockSyncContext( session, hostname,
namedLockFactory, time, timeUnit, shared );
+ }
+
+ public void shutdown()
+ {
+ namedLockFactory.shutdown();
+ }
+
+ private static class AdaptedLockSyncContext
+ implements SyncContext
+ {
+ private static final String CONFIG_PROP_DISCRIMINATOR =
"aether.syncContext.named.discriminator";
+
+ private static final String KEY_PREFIX = "mvn:resolver:";
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(
AdaptedLockSyncContext.class );
+
+ private final RepositorySystemSession session;
+
+ private final String hostname;
+
+ private final NamedLockFactory namedLockFactory;
+
+ private final long time;
+
+ private final TimeUnit timeUnit;
+
+ private final boolean shared;
+
+ private final ArrayDeque<NamedLock> locks;
+
+ private AdaptedLockSyncContext( final RepositorySystemSession session,
+ final String hostname,
+ final NamedLockFactory
namedLockFactory,
+ final long time,
+ final TimeUnit timeUnit,
+ final boolean shared )
+ {
+ this.session = session;
+ this.hostname = hostname;
+ this.namedLockFactory = namedLockFactory;
+ this.time = time;
+ this.timeUnit = timeUnit;
+ this.shared = shared;
+ this.locks = new ArrayDeque<>();
+ }
+
+ @Override
+ public void acquire( Collection<? extends Artifact> artifacts,
+ Collection<? extends Metadata> metadatas )
+ {
+ // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+ // We must acquire multiple locks always in the same order!
+ Collection<String> keys = new TreeSet<>();
+ if ( artifacts != null )
+ {
+ for ( Artifact artifact : artifacts )
+ {
+ // TODO Should we include extension and classifier too?
+ String key = "artifact:" + artifact.getGroupId() + ":"
+ + artifact.getArtifactId() + ":" +
artifact.getBaseVersion();
+ keys.add( key );
+ }
+ }
+
+ if ( metadatas != null )
+ {
+ for ( Metadata metadata : metadatas )
+ {
+ StringBuilder key = new StringBuilder( "metadata:" );
+ if ( !metadata.getGroupId().isEmpty() )
+ {
+ key.append( metadata.getGroupId() );
+ if ( !metadata.getArtifactId().isEmpty() )
+ {
+ key.append( ':' ).append( metadata.getArtifactId()
);
+ if ( !metadata.getVersion().isEmpty() )
+ {
+ key.append( ':' ).append(
metadata.getVersion() );
+ }
+ }
+ }
+ keys.add( key.toString() );
+ }
+ }
+
+ if ( keys.isEmpty() )
+ {
+ return;
+ }
+
+ String discriminator = createDiscriminator();
+ LOGGER.trace( "Using key discriminator '{}' during this session",
discriminator );
+
+ LOGGER.trace( "Need {} {} lock(s) for {}", keys.size(), shared ?
"read" : "write", keys );
+ int acquiredLockCount = 0;
+ for ( String key : keys )
+ {
+ NamedLock namedLock = namedLockFactory.getLock( KEY_PREFIX +
discriminator + ":" + key );
+ try
+ {
+ boolean locked;
+ if ( shared )
+ {
+ locked = namedLock.lockShared( time, timeUnit );
+ }
+ else
+ {
+ locked = namedLock.lockExclusively( time, timeUnit );
+ }
+
+ if ( !locked )
+ {
+ namedLock.close();
+ throw new IllegalStateException( "Could not lock "
+ + namedLock.name() + " (shared=" + shared +
")" );
+ }
+
+ locks.push( namedLock );
+ acquiredLockCount++;
+ }
+ catch ( InterruptedException e )
+ {
+ Thread.currentThread().interrupt();
+ throw new RuntimeException( e );
+ }
+ }
+ LOGGER.trace( "Total new locks acquired: {}", acquiredLockCount );
Review comment:
This isn't true anymore. The reacquisition of locks is gone while from a
debugging point of view I'd like to retain it.
##########
File path: maven-resolver-named-redisson/src/site/site.xml
##########
@@ -22,7 +22,7 @@ under the License.
<project xmlns="http://maven.apache.org/DECORATION/1.0.0";
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
xsi:schemaLocation="http://maven.apache.org/DECORATION/1.0.0
http://maven.apache.org/xsd/decoration-1.0.0.xsd";
- name="RedissonSyncContext">
+ name="NamedSyncContext">
Review comment:
Same as with Hazelcast.
##########
File path: maven-resolver-named/src/site/markdown/index.md.vm
##########
@@ -0,0 +1,69 @@
+${esc.hash} Named Sync Context for Maven Resolver
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+The Named Sync Context Factory is a distributed lock factory for Maven Resolver
+on top of JVM and Hazelcast to provide a fast, concurrent-safe access from one
or multiple Maven instances to the
+same local Maven repository.
+
+For further details about the factory read the
[Javadoc](./apidocs/org/eclipse/aether/synccontext/NamedSyncContextFactory.html).
+
+${esc.hash}${esc.hash} Open Issues/Notes
+
+- It only works when dependency injection is used and not the bundled
`AetherModule` or
+ `ServiceLocator` (Maven uses dependency injection).
+- It includes a lot of trace logging which partially will go way as soon as it
has been stabilized.
+- Usage from plugins has not been tested yet.
+- The `furnace-maven-plugin` does not work this implementation because it uses
`ServiceLocator` instead
+ of dependency injection.
+
+${esc.hash}${esc.hash} Installation/Testing
+
+- Create the directory `${maven.home}/lib/ext/hazelcast/`.
Review comment:
Same as the other file...
##########
File path: maven-resolver-named-hazelcast/src/test/resources/hazelcast.xml
##########
@@ -0,0 +1,42 @@
+<?xml version="1.0"?>
+
+<!--
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+-->
+
+<hazelcast xmlns="http://www.hazelcast.com/schema/config";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://www.hazelcast.com/schema/config
+ http://www.hazelcast.com/schema/config/hazelcast-config-3.12.xsd";>
+
+ <group>
+ <name>maven-resolved-named</name>
+ </group>
+ <instance-name>server</instance-name>
+ <properties>
+ <property name="hazelcast.logging.type">slf4j</property>
+ </properties>
+ <network>
+ <join>
+ <multicast enabled="false"/>
+ <tcp-ip>
+ <interface>127.0.0.1</interface>
Review comment:
Same here
##########
File path:
maven-resolver-named/src/main/java/org/eclipse/aether/named/providers/GlobalReadWriteLockProvider.java
##########
@@ -0,0 +1,129 @@
+package org.eclipse.aether.named.providers;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+
+import javax.inject.Named;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import java.util.ArrayDeque;
+import java.util.NoSuchElementException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+/**
+ * Provider of {@link GlobalNamedLockFactory}. This uses one "global" {@link
ReentrantReadWriteLock}, is really not a
+ * "named" lock.
+ */
+@Singleton
+@Named( GlobalReadWriteLockProvider.NAME )
+public final class GlobalReadWriteLockProvider
+ implements Provider<NamedLockFactory>
+{
+ public static final String NAME = "global";
+
+ @Override
+ public NamedLockFactory get()
+ {
+ return new GlobalNamedLockFactory();
+ }
+
+ /**
+ * A global named lock factory that actually uses one singleton named lock
instance for all names.
+ */
+ public static final class GlobalNamedLockFactory implements
NamedLockFactory
+ {
+ private final GlobalNamedLock globalNamedLock = new GlobalNamedLock();
+
+ @Override
+ public NamedLock getLock( String name )
+ {
+ return globalNamedLock;
+ }
+
+ @Override
+ public void shutdown()
+ {
+ // nop
+ }
+ }
+
+ /**
+ * A global named lock that uses one single reentrant read-write lock.
+ */
+ public static final class GlobalNamedLock implements NamedLock
+ {
+ private final ReentrantReadWriteLock global = new
ReentrantReadWriteLock();
+
+ private final ArrayDeque<Lock> steps = new ArrayDeque<>();
Review comment:
Does the deque serve as hold count for reeentrant calls like in my
Redisson?
```
while ( actualLock.getHoldCount() > 0 )
{
// Avoid #getHoldCount() roundtrips when we are not
logging
if ( LOGGER.isTraceEnabled() )
{
LOGGER.trace( "Releasing {} lock for '{}' (currently
held: {})",
shared ? "read" : "write", key,
actualLock.getHoldCount() );
}
actualLock.unlock();
}
```
##########
File path:
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/SyncContextFactoryAdapter.java
##########
@@ -0,0 +1,272 @@
+package org.eclipse.aether.internal.impl.synccontext;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.SyncContext;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.named.NamedLock;
+import org.eclipse.aether.named.NamedLockFactory;
+import org.eclipse.aether.util.ChecksumUtils;
+import org.eclipse.aether.util.ConfigUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayDeque;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.TreeSet;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Adapter to adapt {@link NamedLockFactory} and {@link NamedLock} to {@link
SyncContext}.
+ */
+public final class SyncContextFactoryAdapter
+{
+ private static final String DEFAULT_DISCRIMINATOR_DIGEST =
"da39a3ee5e6b4b0d3255bfef95601890afd80709";
+
+ private static final String DEFAULT_HOSTNAME = "localhost";
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(
SyncContextFactoryAdapter.class );
+
+ private final NamedLockFactory namedLockFactory;
+
+ private final long time;
+
+ private final TimeUnit timeUnit;
+
+ private final String hostname;
+
+ public SyncContextFactoryAdapter( final NamedLockFactory namedLockFactory,
+ final long time,
+ final TimeUnit timeUnit )
+ {
+ this.namedLockFactory = namedLockFactory;
+ this.time = time;
+ this.timeUnit = timeUnit;
+ this.hostname = getHostname();
+ }
+
+ private String getHostname()
+ {
+ try
+ {
+ return InetAddress.getLocalHost().getHostName();
+ }
+ catch ( UnknownHostException e )
+ {
+ LOGGER.warn( "Failed to get hostname, using '{}'",
DEFAULT_HOSTNAME, e );
+ return DEFAULT_HOSTNAME;
+ }
+ }
+
+ public SyncContext newInstance( final RepositorySystemSession session,
final boolean shared )
+ {
+ return new AdaptedLockSyncContext( session, hostname,
namedLockFactory, time, timeUnit, shared );
+ }
+
+ public void shutdown()
+ {
+ namedLockFactory.shutdown();
+ }
+
+ private static class AdaptedLockSyncContext
+ implements SyncContext
+ {
+ private static final String CONFIG_PROP_DISCRIMINATOR =
"aether.syncContext.named.discriminator";
+
+ private static final String KEY_PREFIX = "mvn:resolver:";
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(
AdaptedLockSyncContext.class );
+
+ private final RepositorySystemSession session;
+
+ private final String hostname;
+
+ private final NamedLockFactory namedLockFactory;
+
+ private final long time;
+
+ private final TimeUnit timeUnit;
+
+ private final boolean shared;
+
+ private final ArrayDeque<NamedLock> locks;
+
+ private AdaptedLockSyncContext( final RepositorySystemSession session,
+ final String hostname,
+ final NamedLockFactory
namedLockFactory,
+ final long time,
+ final TimeUnit timeUnit,
+ final boolean shared )
+ {
+ this.session = session;
+ this.hostname = hostname;
+ this.namedLockFactory = namedLockFactory;
+ this.time = time;
+ this.timeUnit = timeUnit;
+ this.shared = shared;
+ this.locks = new ArrayDeque<>();
+ }
+
+ @Override
+ public void acquire( Collection<? extends Artifact> artifacts,
+ Collection<? extends Metadata> metadatas )
+ {
+ // Deadlock prevention: https://stackoverflow.com/a/16780988/696632
+ // We must acquire multiple locks always in the same order!
+ Collection<String> keys = new TreeSet<>();
+ if ( artifacts != null )
+ {
+ for ( Artifact artifact : artifacts )
+ {
+ // TODO Should we include extension and classifier too?
+ String key = "artifact:" + artifact.getGroupId() + ":"
+ + artifact.getArtifactId() + ":" +
artifact.getBaseVersion();
+ keys.add( key );
+ }
+ }
+
+ if ( metadatas != null )
+ {
+ for ( Metadata metadata : metadatas )
+ {
+ StringBuilder key = new StringBuilder( "metadata:" );
+ if ( !metadata.getGroupId().isEmpty() )
+ {
+ key.append( metadata.getGroupId() );
+ if ( !metadata.getArtifactId().isEmpty() )
+ {
+ key.append( ':' ).append( metadata.getArtifactId()
);
+ if ( !metadata.getVersion().isEmpty() )
+ {
+ key.append( ':' ).append(
metadata.getVersion() );
+ }
+ }
+ }
+ keys.add( key.toString() );
+ }
+ }
+
+ if ( keys.isEmpty() )
+ {
+ return;
+ }
+
+ String discriminator = createDiscriminator();
Review comment:
I think having this here is wrong. This is an inplementation detail of a
concrete named provider and not a general one. Mostly distirbuted locking
systems need it. In-VM don't require it. I would prefer that this classes uses
external lock names while a named provider can tranform this to a internal one.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
