You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/11/04 08:54:42 UTC
[GitHub] [maven-resolver] cstamas opened a new pull request, #216: [MRESOLVER-285] File locking tweaks for Windows OS
cstamas opened a new pull request, #216:
URL: https://github.com/apache/maven-resolver/pull/216
That has concurrency issue at FS level when it comes to deletion of files.
This PR adds several "tweak" flags that allow users on Windows to circumvent the issue.
Related Java bug: https://bugs.openjdk.org/browse/JDK-8252883
---
https://issues.apache.org/jira/browse/MRESOLVER-285
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] gnodet commented on a diff in pull request #216: [MRESOLVER-285] File locking tweaks for Windows OS
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #216:
URL: https://github.com/apache/maven-resolver/pull/216#discussion_r1015982329
##########
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/FileLockNamedLockFactory.java:
##########
@@ -65,11 +95,45 @@ protected NamedLockSupport createLock( final String name )
try
{
Files.createDirectories( path.getParent() );
- return FileChannel.open(
- path,
- StandardOpenOption.READ, StandardOpenOption.WRITE,
- StandardOpenOption.CREATE, StandardOpenOption.DELETE_ON_CLOSE
- );
+ FileChannel channel = retry( ATTEMPTS, SLEEP_MILLIS, () ->
+ {
+ try
+ {
+ if ( DELETE_LOCK_FILES )
+ {
+ return FileChannel.open(
+ path,
+ StandardOpenOption.READ, StandardOpenOption.WRITE,
+ StandardOpenOption.CREATE, StandardOpenOption.DELETE_ON_CLOSE
+ );
+ }
+ else
+ {
+ return FileChannel.open(
+ path,
+ StandardOpenOption.READ, StandardOpenOption.WRITE,
+ StandardOpenOption.CREATE
+ );
+ }
+ }
+ catch ( AccessDeniedException e )
+ {
+ return null;
+ }
+ }, null, null );
+
+ if ( channel == null )
+ {
+ throw new IllegalStateException( "Could not open file channel for '"
+ + name + "' after " + ATTEMPTS + " attempts; giving up" );
+ }
+ return channel;
+ }
+ catch ( InterruptedException e )
+ {
+ Thread.currentThread().interrupt();
+ throw new RuntimeException( "Interrupted during open file channel for '"
Review Comment:
during -> while
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas merged pull request #216: [MRESOLVER-285] File locking tweaks for Windows OS
Posted by GitBox <gi...@apache.org>.
cstamas merged PR #216:
URL: https://github.com/apache/maven-resolver/pull/216
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #216: [MRESOLVER-285] File locking tweaks for Windows OS
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #216:
URL: https://github.com/apache/maven-resolver/pull/216#discussion_r1014375888
##########
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/FileLockNamedLockFactory.java:
##########
@@ -49,6 +52,33 @@
{
public static final String NAME = "file-lock";
+ /**
+ * Tweak: on Windows, the presence of {@link StandardOpenOption#DELETE_ON_CLOSE} causes concurrency issues. This
+ * flag allows to have it removed from effective flags, at the cost that lockfile directory becomes crowded
+ * with 0 byte sized lock files that are never cleaned up. Default value is {@code true}.
+ *
+ * @see <a href="https://bugs.openjdk.org/browse/JDK-8252883">JDK-8252883</a>
+ */
+ private static final boolean DELETE_LOCK_FILES = Boolean.parseBoolean(
+ System.getProperty( "aether.named.file-lock.deleteLockFiles", Boolean.TRUE.toString() ) );
+
+ /**
+ * Tweak: on Windows, the presence of {@link StandardOpenOption#DELETE_ON_CLOSE} causes concurrency issues. This
+ * flag allows to implement similar fix as referenced JDK bug report: retry and hope the best. Default value is
+ * 5 attempts (will retry 4 times).
+ *
+ * @see <a href="https://bugs.openjdk.org/browse/JDK-8252883">JDK-8252883</a>
+ */
+ private static final int ATTEMPTS = Integer.parseInt(
+ System.getProperty( "aether.named.file-lock.attempts", "5" ) );
+
+ /**
+ * Tweak: When {@link #ATTEMPTS} used, the amount of milliseconds to sleep between subsequent retries. Default
+ * value is 50 milliseconds.
+ */
+ private static final long SLEEP_MILLIS = Long.parseLong(
+ System.getProperty( "aether.named.file-lock.sleepMillis", "50" ) );
+
Review Comment:
These are "tweaks", something very few should touch at all (as testing shows is fixing the issue). I still wanted to expose some parameters to tuning... As that may allow users to apply some changes without recompile sources.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] michael-o commented on a diff in pull request #216: [MRESOLVER-285] File locking tweaks for Windows OS
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #216:
URL: https://github.com/apache/maven-resolver/pull/216#discussion_r1015855274
##########
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/FileLockNamedLockFactory.java:
##########
@@ -65,11 +95,45 @@ protected NamedLockSupport createLock( final String name )
try
{
Files.createDirectories( path.getParent() );
- return FileChannel.open(
- path,
- StandardOpenOption.READ, StandardOpenOption.WRITE,
- StandardOpenOption.CREATE, StandardOpenOption.DELETE_ON_CLOSE
- );
+ FileChannel channel = retry( ATTEMPTS, SLEEP_MILLIS, () ->
+ {
+ try
+ {
+ if ( DELETE_LOCK_FILES )
+ {
+ return FileChannel.open(
+ path,
+ StandardOpenOption.READ, StandardOpenOption.WRITE,
+ StandardOpenOption.CREATE, StandardOpenOption.DELETE_ON_CLOSE
+ );
+ }
+ else
+ {
+ return FileChannel.open(
+ path,
+ StandardOpenOption.READ, StandardOpenOption.WRITE,
+ StandardOpenOption.CREATE
+ );
+ }
+ }
+ catch ( AccessDeniedException e )
+ {
+ return null;
+ }
+ }, null, null );
+
+ if ( channel == null )
+ {
+ throw new IllegalStateException( "Could not open file channel for '"
+ + name + "' after " + ATTEMPTS + " attempts; giving up" );
+ }
+ return channel;
+ }
+ catch ( InterruptedException e )
+ {
+ Thread.currentThread().interrupt();
+ throw new RuntimeException( "Interrupted during open file channel for '"
Review Comment:
opening
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on pull request #216: [MRESOLVER-285] File locking tweaks for Windows OS
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #216:
URL: https://github.com/apache/maven-resolver/pull/216#issuecomment-1303159372
Personally, as BOTH tweaks work, I'd remove the `DELETE_LOCK_FILES` tweak.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #216: [MRESOLVER-285] File locking tweaks for Windows OS
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #216:
URL: https://github.com/apache/maven-resolver/pull/216#discussion_r1014407116
##########
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/FileLockNamedLockFactory.java:
##########
@@ -49,6 +52,33 @@
{
public static final String NAME = "file-lock";
+ /**
+ * Tweak: on Windows, the presence of {@link StandardOpenOption#DELETE_ON_CLOSE} causes concurrency issues. This
+ * flag allows to have it removed from effective flags, at the cost that lockfile directory becomes crowded
+ * with 0 byte sized lock files that are never cleaned up. Default value is {@code true}.
+ *
+ * @see <a href="https://bugs.openjdk.org/browse/JDK-8252883">JDK-8252883</a>
+ */
+ private static final boolean DELETE_LOCK_FILES = Boolean.parseBoolean(
+ System.getProperty( "aether.named.file-lock.deleteLockFiles", Boolean.TRUE.toString() ) );
+
+ /**
+ * Tweak: on Windows, the presence of {@link StandardOpenOption#DELETE_ON_CLOSE} causes concurrency issues. This
+ * flag allows to implement similar fix as referenced JDK bug report: retry and hope the best. Default value is
+ * 5 attempts (will retry 4 times).
+ *
+ * @see <a href="https://bugs.openjdk.org/browse/JDK-8252883">JDK-8252883</a>
+ */
+ private static final int ATTEMPTS = Integer.parseInt(
+ System.getProperty( "aether.named.file-lock.attempts", "5" ) );
+
+ /**
+ * Tweak: When {@link #ATTEMPTS} used, the amount of milliseconds to sleep between subsequent retries. Default
+ * value is 50 milliseconds.
+ */
+ private static final long SLEEP_MILLIS = Long.parseLong(
+ System.getProperty( "aether.named.file-lock.sleepMillis", "50" ) );
+
Review Comment:
Also, adapter (hence, factory) is created way before session.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on pull request #216: [MRESOLVER-285] File locking tweaks for Windows OS
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #216:
URL: https://github.com/apache/maven-resolver/pull/216#issuecomment-1303155651
Both "tweaks" are reported as "works for me" on Windows by user, who was able consistently to reproduce issue https://github.com/apache/maven-mvnd/issues/728
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] michael-o commented on a diff in pull request #216: [MRESOLVER-285] File locking tweaks for Windows OS
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #216:
URL: https://github.com/apache/maven-resolver/pull/216#discussion_r1014355905
##########
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/FileLockNamedLockFactory.java:
##########
@@ -49,6 +52,33 @@
{
public static final String NAME = "file-lock";
+ /**
+ * Tweak: on Windows, the presence of {@link StandardOpenOption#DELETE_ON_CLOSE} causes concurrency issues. This
+ * flag allows to have it removed from effective flags, at the cost that lockfile directory becomes crowded
+ * with 0 byte sized lock files that are never cleaned up. Default value is {@code true}.
+ *
+ * @see <a href="https://bugs.openjdk.org/browse/JDK-8252883">JDK-8252883</a>
+ */
+ private static final boolean DELETE_LOCK_FILES = Boolean.parseBoolean(
+ System.getProperty( "aether.named.file-lock.deleteLockFiles", Boolean.TRUE.toString() ) );
+
+ /**
+ * Tweak: on Windows, the presence of {@link StandardOpenOption#DELETE_ON_CLOSE} causes concurrency issues. This
+ * flag allows to implement similar fix as referenced JDK bug report: retry and hope the best. Default value is
+ * 5 attempts (will retry 4 times).
+ *
+ * @see <a href="https://bugs.openjdk.org/browse/JDK-8252883">JDK-8252883</a>
+ */
+ private static final int ATTEMPTS = Integer.parseInt(
+ System.getProperty( "aether.named.file-lock.attempts", "5" ) );
+
+ /**
+ * Tweak: When {@link #ATTEMPTS} used, the amount of milliseconds to sleep between subsequent retries. Default
+ * value is 50 milliseconds.
+ */
+ private static final long SLEEP_MILLIS = Long.parseLong(
+ System.getProperty( "aether.named.file-lock.sleepMillis", "50" ) );
+
Review Comment:
Now those are system properties and not session properties. Any reason?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org