Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong usage of Caffeine cache #180

Closed
infeo opened this issue Nov 20, 2023 · 0 comments · Fixed by #181
Closed

Wrong usage of Caffeine cache #180

infeo opened this issue Nov 20, 2023 · 0 comments · Fixed by #181
Assignees
Labels
Milestone

Comments

@infeo
Copy link
Member

infeo commented Nov 20, 2023

With PR #164 we switched our caching library from Google's Guave implementation to Caffeine.

Unfortunately, caffeine does not allow recursive updates of the cache (see https://github.com/ben-manes/caffeine/wiki/Faq#recursive-computations), but we are doing it anyway in the CryptoPathMapper class:

	public CiphertextFilePath getCiphertextFilePath(CryptoPath cleartextPath) throws IOException {
		CryptoPath parentPath = cleartextPath.getParent();
		if (parentPath == null) {
			throw new IllegalArgumentException("Invalid file path (must have a parent): " + cleartextPath);
		}
//---------HERE: we call getCiphertextDir(...)
		CiphertextDirectory parent = getCiphertextDir(parentPath);
		String cleartextName = cleartextPath.getFileName().toString();
		return getCiphertextFilePath(parent.path, parent.dirId, cleartextName);
	}


	public CiphertextDirectory getCiphertextDir(CryptoPath cleartextPath) throws IOException {
		assert cleartextPath.isAbsolute();
		CryptoPath parentPath = cleartextPath.getParent();
		if (parentPath == null) {
			return rootDirectory;
		} else {
			try {
//--------------------cache.get() is an if-cached-return-else-create-and-return operation
				return ciphertextDirectories.get(cleartextPath, p -> {
					try {
//-------------------------------HERE: we call getCiphertextFilepath(...)
						Path dirFile = getCiphertextFilePath(p).getDirFilePath();
						return resolveDirectory(dirFile);
					} catch (IOException e) {
						throw new UncheckedIOException(e);
					}
				});
			} catch (UncheckedIOException e) {
				throw new IOException(e);
			}
		}
	}

This leads to the following stacktrace when accessing deeper folderstructures:

java.lang.IllegalStateException: Recursive update

	at java.base/java.util.concurrent.ConcurrentHashMap.compute(ConcurrentHashMap.java:1991)
	at com.github.benmanes.caffeine/com.github.benmanes.caffeine.cache.BoundedLocalCache.doComputeIfAbsent(BoundedLocalCache.java:2666)
	at com.github.benmanes.caffeine/com.github.benmanes.caffeine.cache.BoundedLocalCache.computeIfAbsent(BoundedLocalCache.java:2649)
	at com.github.benmanes.caffeine/com.github.benmanes.caffeine.cache.LocalCache.computeIfAbsent(LocalCache.java:112)
	at com.github.benmanes.caffeine/com.github.benmanes.caffeine.cache.LocalManualCache.get(LocalManualCache.java:62)
	at org.cryptomator.cryptofs/org.cryptomator.cryptofs.CryptoPathMapper.getCiphertextDir(CryptoPathMapper.java:161)
	at org.cryptomator.cryptofs/org.cryptomator.cryptofs.CryptoPathMapper.getCiphertextFilePath(CryptoPathMapper.java:122)
....//more repetetions of the above stacktrace
	at org.cryptomator.cryptofs/org.cryptomator.cryptofs.CryptoPathMapper.lambda$getCiphertextDir$0(CryptoPathMapper.java:163)
	at com.github.benmanes.caffeine/com.github.benmanes.caffeine.cache.BoundedLocalCache.lambda$doComputeIfAbsent$14(BoundedLocalCache.java:2668)
	at java.base/java.util.concurrent.ConcurrentHashMap.compute(ConcurrentHashMap.java:1916)
	at com.github.benmanes.caffeine/com.github.benmanes.caffeine.cache.BoundedLocalCache.doComputeIfAbsent(BoundedLocalCache.java:2666)
	at com.github.benmanes.caffeine/com.github.benmanes.caffeine.cache.BoundedLocalCache.computeIfAbsent(BoundedLocalCache.java:2649)
	at com.github.benmanes.caffeine/com.github.benmanes.caffeine.cache.LocalCache.computeIfAbsent(LocalCache.java:112)
	at com.github.benmanes.caffeine/com.github.benmanes.caffeine.cache.LocalManualCache.get(LocalManualCache.java:62)
	at org.cryptomator.cryptofs/org.cryptomator.cryptofs.CryptoPathMapper.getCiphertextDir(CryptoPathMapper.java:161)
	at org.cryptomator.cryptofs/org.cryptomator.cryptofs.CryptoPathMapper.getCiphertextFilePath(CryptoPathMapper.java:122)
	at org.cryptomator.cryptofs/org.cryptomator.cryptofs.CryptoPathMapperTest.testPathEncryptionDEEP(CryptoPathMapperTest.java:173)
@infeo infeo self-assigned this Nov 20, 2023
@infeo infeo added the bug label Nov 20, 2023
@infeo infeo added this to the 2.6.8 milestone Nov 20, 2023
infeo added a commit that referenced this issue Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant