From 702db13af2b96c0f699fa6be0a63ba281535f337 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sun, 1 Dec 2024 21:59:03 +0100 Subject: UploadPack#implies: add missing @since tag The method was originally introduced in 7.1 and then back ported to the stable-6.10 branch. Change-Id: I7250013227a666de44e439802d51110bba5af2b9 --- org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'org.eclipse.jgit/src') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index e9ea9b25a6..968728de8d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -156,8 +156,10 @@ public class UploadPack implements Closeable { /** * Check if the current policy implies another, based on its bitmask. * - * @param implied the implied policy based on its bitmask. + * @param implied + * the implied policy based on its bitmask. * @return true if the policy is implied. + * @since 6.10.1 */ public boolean implies(RequestPolicy implied) { return (bitmask & implied.bitmask) != 0; -- cgit v1.2.3 From 44d61a3d727eb68db9e57acec0704bfc6758f878 Mon Sep 17 00:00:00 2001 From: Martin Fick Date: Wed, 27 Nov 2024 10:15:30 -0800 Subject: WindowCache: add bulk purge(), call from bulk sites Purging a Pack from the WindowCache requires a linear search over all the entries in the cache, and thus is rather expensive. Since git gc often deletes more than one packfile at a time, avoid multiplying this expensive operation when possible by purging a Set of Packs when closing deleted pack files during a directory scan. Purge the Set of Packs with a single linear search of the cache. Closing the PackDirectory also cccbngljkihltghcnbiftcubdvgugdcvujkejehbjr Change-Id: Ic9b45cab57e1ef610c2e20ad392d8b45f8091f41 Signed-off-by: Martin Fick --- .../eclipse/jgit/internal/storage/file/Pack.java | 25 ++++++++++++++++------ .../jgit/internal/storage/file/PackDirectory.java | 9 +++----- .../jgit/internal/storage/file/WindowCache.java | 24 +++++++++++++-------- 3 files changed, 37 insertions(+), 21 deletions(-) (limited to 'org.eclipse.jgit/src') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java index 3518342f97..9073c1675a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java @@ -296,15 +296,28 @@ public class Pack implements Iterable { } /** - * Close the resources utilized by this repository + * Close the resources utilized by these pack files + * + * @param packs + * packs to close + */ + public static void close(Set packs) { + WindowCache.purge(packs); + packs.forEach(p -> p.closeIndices()); + } + + /** + * Close the resources utilized by this pack file */ public void close() { WindowCache.purge(this); - synchronized (this) { - loadedIdx.clear(); - reverseIdx.clear(); - bitmapIdx.clear(); - } + closeIndices(); + } + + private synchronized void closeIndices() { + loadedIdx.clear(); + reverseIdx.clear(); + bitmapIdx.clear(); } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java index 51a8148838..e31126f027 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java @@ -24,6 +24,7 @@ import java.util.Collection; import java.util.Collections; import java.util.EnumMap; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -111,9 +112,7 @@ class PackDirectory { void close() { PackList packs = packList.get(); if (packs != NO_PACKS && packList.compareAndSet(packs, NO_PACKS)) { - for (Pack p : packs.packs) { - p.close(); - } + Pack.close(Set.of(packs.packs)); } } @@ -484,9 +483,7 @@ class PackDirectory { return old; } - for (Pack p : forReuse.values()) { - p.close(); - } + Pack.close(new HashSet<>(forReuse.values())); if (list.isEmpty()) { return new PackList(snapshot, NO_PACKS.packs); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java index 30f8240aa9..3ec7e6e666 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/WindowCache.java @@ -17,6 +17,7 @@ import java.lang.ref.SoftReference; import java.util.Collections; import java.util.Map; import java.util.Random; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicBoolean; @@ -397,7 +398,11 @@ public class WindowCache { } static final void purge(Pack pack) { - cache.removeAll(pack); + purge(Collections.singleton(pack)); + } + + static final void purge(Set packs) { + cache.removeAll(packs); } /** cleanup released and/or garbage collected windows. */ @@ -710,20 +715,21 @@ public class WindowCache { /** * Clear all entries related to a single file. *

- * Typically this method is invoked during {@link Pack#close()}, when we - * know the pack is never going to be useful to us again (for example, it no - * longer exists on disk). A concurrent reader loading an entry from this - * same pack may cause the pack to become stuck in the cache anyway. + * Typically this method is invoked during {@link Pack#close()}, or + * {@link Pack#close(Set)}, when we know the packs are never going to be + * useful to us again (for example, they no longer exist on disk after gc). + * A concurrent reader loading an entry from these same packs may cause a + * pack to become stuck in the cache anyway. * - * @param pack - * the file to purge all entries of. + * @param packs + * the files to purge all entries of. */ - private void removeAll(Pack pack) { + private void removeAll(Set packs) { for (int s = 0; s < tableSize; s++) { final Entry e1 = table.get(s); boolean hasDead = false; for (Entry e = e1; e != null; e = e.next) { - if (e.ref.getPack() == pack) { + if (packs.contains(e.ref.getPack())) { e.kill(); hasDead = true; } else if (e.dead) -- cgit v1.2.3 From fb6adb0366bc3e15f30952fe49456b1f96bcf1af Mon Sep 17 00:00:00 2001 From: Martin Fick Date: Fri, 22 Nov 2024 17:57:17 -0800 Subject: Pack: ensure packfile is still valid while still recoverable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When copyAsIs2() needs to send a large object requiring multiple read iterations — any of which could fail if the object isn't already in the WindowCache — verify first that the packfile is still valid at the latest possible moment, just before sending the header that would make recovery impossible. Change-Id: I234fd4b315b579a0506c8fbdea0c6787bdc09fcd Signed-off-by: Martin Fick --- .../src/org/eclipse/jgit/internal/storage/file/Pack.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'org.eclipse.jgit/src') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java index 9073c1675a..61577a956c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java @@ -597,13 +597,19 @@ public class Pack implements Iterable { assert(crc2 != null); crc2.update(buf, 0, n); } + cnt -= n; if (!isHeaderWritten) { + if (invalid && cnt > 0) { + // Since this is not the last iteration and the packfile is invalid, + // better to assume the iterations will not all complete here while + // it is still likely recoverable. + throw new StoredObjectRepresentationNotAvailableException(invalidatingCause); + } out.writeHeader(src, inflatedLength); isHeaderWritten = true; } out.write(buf, 0, n); pos += n; - cnt -= n; } if (validate) { assert(crc2 != null); -- cgit v1.2.3 From f2741cacec9f46724a0e48cc55cbb8892361e3d0 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 4 Dec 2024 10:34:10 +0100 Subject: Fix NPE in DiffFormatter#getDiffDriver Guard against diff attribute with a null value. Change-Id: Icc4b8d2f360ef105c6d64716cb42f2979967075b --- .../src/org/eclipse/jgit/diff/DiffFormatter.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'org.eclipse.jgit/src') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java index fa446e18cd..6375a60383 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java @@ -1356,13 +1356,17 @@ public class DiffFormatter implements AutoCloseable { private DiffDriver getDiffDriver(DiffEntry entry) { Attribute diffAttr = entry.getDiffAttribute(); - if (diffAttr != null) { - try { - return DiffDriver.valueOf(diffAttr.getValue()); - } catch (IllegalArgumentException e) { - return null; - } + if (diffAttr == null) { + return null; + } + String diffAttrValue = diffAttr.getValue(); + if (diffAttrValue == null) { + return null; + } + try { + return DiffDriver.valueOf(diffAttrValue); + } catch (IllegalArgumentException e) { + return null; } - return null; } } -- cgit v1.2.3 From 7f246a05dcbf11a6c9dcd21215f38f62ad801866 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 4 Dec 2024 11:19:39 +0100 Subject: Fix potential NPE in ResolveMerger#getAttributesContentMergeStrategy Change-Id: I2a4c57438c16a0c5bc1af4c7772eaf65049911e2 --- org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'org.eclipse.jgit/src') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java index a6a287bcf4..a50a6440b9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java @@ -1507,9 +1507,12 @@ public class ResolveMerger extends ThreeWayMerger { private ContentMergeStrategy getAttributesContentMergeStrategy( Attributes attributes, ContentMergeStrategy strategy) { Attribute attr = attributes.get(Constants.ATTR_MERGE); - if (attr != null && attr.getValue() - .equals(Constants.ATTR_BUILTIN_UNION_MERGE_DRIVER)) { - return ContentMergeStrategy.UNION; + if (attr != null) { + String attrValue = attr.getValue(); + if (attrValue != null && attrValue + .equals(Constants.ATTR_BUILTIN_UNION_MERGE_DRIVER)) { + return ContentMergeStrategy.UNION; + } } return strategy; } -- cgit v1.2.3 From 4156bdfe8ef8af2cc1d48a185e62fdf83e14c8ed Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Wed, 4 Dec 2024 11:20:19 +0100 Subject: Mark Attribute#getValue as @Nullable Change-Id: I172c43ff2e3e682f38a91ce288190245fa5d5ebe --- org.eclipse.jgit/src/org/eclipse/jgit/attributes/Attribute.java | 3 +++ 1 file changed, 3 insertions(+) (limited to 'org.eclipse.jgit/src') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/attributes/Attribute.java b/org.eclipse.jgit/src/org/eclipse/jgit/attributes/Attribute.java index fe3e22a21f..9c4d8700a2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/attributes/Attribute.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/attributes/Attribute.java @@ -9,6 +9,8 @@ */ package org.eclipse.jgit.attributes; +import org.eclipse.jgit.annotations.Nullable; + /** * Represents an attribute. *

@@ -139,6 +141,7 @@ public final class Attribute { * * @return the attribute value (may be null) */ + @Nullable public String getValue() { return value; } -- cgit v1.2.3 From dd8b13acc0d60b53f399ce7e0c6718b81ad2ab16 Mon Sep 17 00:00:00 2001 From: Martin Fick Date: Thu, 5 Dec 2024 13:37:37 -0800 Subject: FileSnapshot: refactor to share error handling It is important to keep the exception handling for getting file attributes the same in all places in this class; put that code into a common method. Change-Id: I1fcce5efd10aa562a4e0e34d3ce94bcc83850237 Signed-off-by: Martin Fick --- .../jgit/internal/storage/file/FileSnapshot.java | 30 +++++++++++----------- 1 file changed, 15 insertions(+), 15 deletions(-) (limited to 'org.eclipse.jgit/src') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java index c88ac984ec..c5e9e99673 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java @@ -22,6 +22,7 @@ import java.time.Instant; import java.time.ZoneId; import java.time.format.DateTimeFormatter; import java.util.Locale; +import java.util.NoSuchElementException; import java.util.Objects; import java.util.concurrent.TimeUnit; @@ -231,14 +232,8 @@ public class FileSnapshot { this.useConfig = useConfig; BasicFileAttributes fileAttributes = null; try { - fileAttributes = FS.DETECTED.fileAttributes(file); - } catch (NoSuchFileException e) { - this.lastModified = Instant.EPOCH; - this.size = 0L; - this.fileKey = MISSING_FILEKEY; - return; - } catch (IOException e) { - LOG.error(e.getMessage(), e); + fileAttributes = getFileAttributes(file); + } catch (NoSuchElementException e) { this.lastModified = Instant.EPOCH; this.size = 0L; this.fileKey = MISSING_FILEKEY; @@ -319,16 +314,11 @@ public class FileSnapshot { long currSize; Object currFileKey; try { - BasicFileAttributes fileAttributes = FS.DETECTED.fileAttributes(path); + BasicFileAttributes fileAttributes = getFileAttributes(path); currLastModified = fileAttributes.lastModifiedTime().toInstant(); currSize = fileAttributes.size(); currFileKey = getFileKey(fileAttributes); - } catch (NoSuchFileException e) { - currLastModified = Instant.EPOCH; - currSize = 0L; - currFileKey = MISSING_FILEKEY; - } catch (IOException e) { - LOG.error(e.getMessage(), e); + } catch (NoSuchElementException e) { currLastModified = Instant.EPOCH; currSize = 0L; currFileKey = MISSING_FILEKEY; @@ -586,4 +576,14 @@ public class FileSnapshot { } return fileStoreAttributeCache; } + + private static BasicFileAttributes getFileAttributes(File path) throws NoSuchElementException { + try { + return FS.DETECTED.fileAttributes(path); + } catch (NoSuchFileException e) { + } catch (IOException e) { + LOG.error(e.getMessage(), e); + } + throw new NoSuchElementException(path.toString()); + } } -- cgit v1.2.3 From eb0ef9d16ea780c431837f904ad4d1ad0aaa5d7d Mon Sep 17 00:00:00 2001 From: Martin Fick Date: Thu, 5 Dec 2024 14:33:34 -0800 Subject: FileSnapshot: silence "Not a Directory" exceptions Sometimes a FileSystemException with "Not a Directory" can be thrown while creating a FileSnapshot, likely because the file or directory was deleted. Since NoSuchFileExceptions are already silenced, and the FileSnapshot already handles all IOExceptions, there is likely no value in seeing this info in the logs, treat these situation the same and silence them also. Change-Id: I022639c42154a0a4c71065741f023f5eb95f9b55 Signed-off-by: Martin Fick --- .../src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'org.eclipse.jgit/src') diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java index c5e9e99673..d34baf358c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java @@ -15,6 +15,7 @@ import static org.eclipse.jgit.util.FS.FileStoreAttributes.FALLBACK_TIMESTAMP_RE import java.io.File; import java.io.IOException; +import java.nio.file.FileSystemException; import java.nio.file.NoSuchFileException; import java.nio.file.attribute.BasicFileAttributes; import java.time.Duration; @@ -581,6 +582,10 @@ public class FileSnapshot { try { return FS.DETECTED.fileAttributes(path); } catch (NoSuchFileException e) { + } catch (FileSystemException e) { + if (!e.getMessage().endsWith("Not a directory")) { + LOG.error(e.getMessage(), e); + } } catch (IOException e) { LOG.error(e.getMessage(), e); } -- cgit v1.2.3