[ASTERIXDB-2398][STO] Ensure No Concurrent Access to FileMapManager
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- Currently it is possible for a thread to access
FileMapManager without holding the proper lock
and therefore seeing a stale state of the map.
This change ensures that all access to the map
is synchronized on the same lock.
Change-Id: Iea7fdb0b891b4ba2aaa528b42eab47b6f841672d
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2863
Sonar-Qube: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: abdullah alamoudi <bamousaa@gmail.com>
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
index f8b9a57..b35cefe 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java
@@ -849,9 +849,8 @@
if (LOGGER.isEnabled(fileOpsLevel)) {
LOGGER.log(fileOpsLevel, "Opening file: " + fileId + " in cache: " + this);
}
- BufferedFileHandle fInfo = null;
try {
- fInfo = getOrCreateFileHandle(fileId);
+ final BufferedFileHandle fInfo = getOrCreateFileHandle(fileId);
if (fInfo.getFileHandle() == null) {
// a new file
synchronized (fInfo) {
@@ -861,7 +860,10 @@
closeOpeningFiles(fInfo);
}
// create, open, and map new file reference
- FileReference fileRef = fileMapManager.lookupFileName(fileId);
+ FileReference fileRef;
+ synchronized (fileInfoMap) {
+ fileRef = fileMapManager.lookupFileName(fileId);
+ }
IFileHandle fh = ioManager.open(fileRef, IIOManager.FileReadWriteMode.READ_WRITE,
IIOManager.FileSyncMode.METADATA_ASYNC_DATA_ASYNC);
fInfo.setFileHandle(fh);
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/BufferedFileHandle.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/BufferedFileHandle.java
index 1312d97..4f15588 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/BufferedFileHandle.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/BufferedFileHandle.java
@@ -24,7 +24,7 @@
public class BufferedFileHandle {
private final int fileId;
- private IFileHandle handle;
+ private volatile IFileHandle handle;
private final AtomicInteger refCount;
public BufferedFileHandle(int fileId, IFileHandle handle) {
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/FileMapManager.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/FileMapManager.java
index 32922d2..0f2703b 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/FileMapManager.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/FileMapManager.java
@@ -25,7 +25,9 @@
import org.apache.hyracks.api.exceptions.ErrorCode;
import org.apache.hyracks.api.exceptions.HyracksDataException;
import org.apache.hyracks.api.io.FileReference;
+import org.apache.hyracks.util.annotations.NotThreadSafe;
+@NotThreadSafe
public class FileMapManager implements IFileMapManager {
private static final long serialVersionUID = 1L;