Browse Source

Thread Syncronization for FileAccessHandler and DirectoryAccessHandler

- Switching to ReentrantLock
- Thread locking for DirectoryAccessHandler (dependencies)

Newline @ EOF

Forgot import

Revert

Better locking

Forgot return

Restore last empty line
h1v9 1 month ago
parent
commit
b787b0dbb5

+ 18 - 15
platform/android/java/lib/src/org/godotengine/godot/io/directory/DirectoryAccessHandler.kt

@@ -35,6 +35,8 @@ import android.util.Log
 import org.godotengine.godot.Godot
 import org.godotengine.godot.io.StorageScope
 import org.godotengine.godot.io.directory.DirectoryAccessHandler.AccessType.ACCESS_RESOURCES
+import java.util.concurrent.locks.ReentrantLock
+import kotlin.concurrent.withLock
 
 /**
  * Handles files and directories access and manipulation for the Android platform
@@ -145,9 +147,10 @@ class DirectoryAccessHandler(context: Context) {
 
 	private val assetsDirAccess = AssetsDirectoryAccess(context)
 	private val fileSystemDirAccess = FilesystemDirectoryAccess(context, storageScopeIdentifier)
+	private val lock = ReentrantLock()
 
-	fun assetsFileExists(assetsPath: String) = assetsDirAccess.fileExists(assetsPath)
-	fun filesystemFileExists(path: String) = fileSystemDirAccess.fileExists(path)
+	fun assetsFileExists(assetsPath: String) = lock.withLock { assetsDirAccess.fileExists(assetsPath) }
+	fun filesystemFileExists(path: String) = lock.withLock { fileSystemDirAccess.fileExists(path) }
 
 	private fun hasDirId(accessType: AccessType, dirId: Int): Boolean {
 		return when (accessType) {
@@ -156,7 +159,7 @@ class DirectoryAccessHandler(context: Context) {
 		}
 	}
 
-	fun dirOpen(nativeAccessType: Int, path: String?): Int {
+	fun dirOpen(nativeAccessType: Int, path: String?): Int = lock.withLock {
 		if (path == null) {
 			return INVALID_DIR_ID
 		}
@@ -176,7 +179,7 @@ class DirectoryAccessHandler(context: Context) {
 		return dirAccessId
 	}
 
-	fun dirNext(dirAccessId: Int): String {
+	fun dirNext(dirAccessId: Int): String = lock.withLock {
 		val (accessType, dirId) = AccessType.fromDirAccessId(dirAccessId)
 		if (accessType == null || !hasDirId(accessType, dirId)) {
 			Log.w(TAG, "dirNext: Invalid dir id: $dirId")
@@ -189,7 +192,7 @@ class DirectoryAccessHandler(context: Context) {
 		}
 	}
 
-	fun dirClose(dirAccessId: Int) {
+	fun dirClose(dirAccessId: Int) = lock.withLock {
 		val (accessType, dirId) = AccessType.fromDirAccessId(dirAccessId)
 		if (accessType == null || !hasDirId(accessType, dirId)) {
 			Log.w(TAG, "dirClose: Invalid dir id: $dirId")
@@ -202,7 +205,7 @@ class DirectoryAccessHandler(context: Context) {
 		}
 	}
 
-	fun dirIsDir(dirAccessId: Int): Boolean {
+	fun dirIsDir(dirAccessId: Int): Boolean = lock.withLock {
 		val (accessType, dirId) = AccessType.fromDirAccessId(dirAccessId)
 		if (accessType == null || !hasDirId(accessType, dirId)) {
 			Log.w(TAG, "dirIsDir: Invalid dir id: $dirId")
@@ -215,7 +218,7 @@ class DirectoryAccessHandler(context: Context) {
 		}
 	}
 
-	fun isCurrentHidden(dirAccessId: Int): Boolean {
+	fun isCurrentHidden(dirAccessId: Int): Boolean = lock.withLock {
 		val (accessType, dirId) = AccessType.fromDirAccessId(dirAccessId)
 		if (accessType == null || !hasDirId(accessType, dirId)) {
 			return false
@@ -227,7 +230,7 @@ class DirectoryAccessHandler(context: Context) {
 		}
 	}
 
-	fun dirExists(nativeAccessType: Int, path: String?): Boolean {
+	fun dirExists(nativeAccessType: Int, path: String?): Boolean = lock.withLock {
 		if (path == null) {
 			return false
 		}
@@ -241,7 +244,7 @@ class DirectoryAccessHandler(context: Context) {
 		}
 	}
 
-	fun fileExists(nativeAccessType: Int, path: String?): Boolean {
+	fun fileExists(nativeAccessType: Int, path: String?): Boolean = lock.withLock {
 		if (path == null) {
 			return false
 		}
@@ -255,7 +258,7 @@ class DirectoryAccessHandler(context: Context) {
 		}
 	}
 
-	fun getDriveCount(nativeAccessType: Int): Int {
+	fun getDriveCount(nativeAccessType: Int): Int = lock.withLock {
 		val accessType = AccessType.fromNative(nativeAccessType) ?: return 0
 		return when(accessType) {
 			ACCESS_RESOURCES -> assetsDirAccess.getDriveCount()
@@ -263,7 +266,7 @@ class DirectoryAccessHandler(context: Context) {
 		}
 	}
 
-	fun getDrive(nativeAccessType: Int, drive: Int): String {
+	fun getDrive(nativeAccessType: Int, drive: Int): String = lock.withLock {
 		val accessType = AccessType.fromNative(nativeAccessType) ?: return ""
 		return when (accessType) {
 			ACCESS_RESOURCES -> assetsDirAccess.getDrive(drive)
@@ -271,7 +274,7 @@ class DirectoryAccessHandler(context: Context) {
 		}
 	}
 
-	fun makeDir(nativeAccessType: Int, dir: String?): Boolean {
+	fun makeDir(nativeAccessType: Int, dir: String?): Boolean = lock.withLock {
 		if (dir == null) {
 			return false
 		}
@@ -285,7 +288,7 @@ class DirectoryAccessHandler(context: Context) {
 		}
 	}
 
-	fun getSpaceLeft(nativeAccessType: Int): Long {
+	fun getSpaceLeft(nativeAccessType: Int): Long = lock.withLock {
 		val accessType = AccessType.fromNative(nativeAccessType) ?: return 0L
 		return when (accessType) {
 			ACCESS_RESOURCES -> assetsDirAccess.getSpaceLeft()
@@ -293,7 +296,7 @@ class DirectoryAccessHandler(context: Context) {
 		}
 	}
 
-	fun rename(nativeAccessType: Int, from: String, to: String): Boolean {
+	fun rename(nativeAccessType: Int, from: String, to: String): Boolean = lock.withLock {
 		val accessType = AccessType.fromNative(nativeAccessType) ?: return false
 		return when (accessType) {
 			ACCESS_RESOURCES -> assetsDirAccess.rename(from, to)
@@ -301,7 +304,7 @@ class DirectoryAccessHandler(context: Context) {
 		}
 	}
 
-	fun remove(nativeAccessType: Int, filename: String?): Boolean {
+	fun remove(nativeAccessType: Int, filename: String?): Boolean = lock.withLock {
 		if (filename == null) {
 			return false
 		}

+ 18 - 14
platform/android/java/lib/src/org/godotengine/godot/io/file/FileAccessHandler.kt

@@ -39,6 +39,8 @@ import java.io.FileNotFoundException
 import java.io.InputStream
 import java.lang.UnsupportedOperationException
 import java.nio.ByteBuffer
+import java.util.concurrent.locks.ReentrantLock
+import kotlin.concurrent.withLock
 
 /**
  * Handles regular and media store file access and interactions.
@@ -110,8 +112,11 @@ class FileAccessHandler(val context: Context) {
 	internal val storageScopeIdentifier = StorageScope.Identifier(context)
 	private val files = SparseArray<DataAccess>()
 	private var lastFileId = STARTING_FILE_ID
+	private val lock = ReentrantLock()
 
-	private fun hasFileId(fileId: Int) = files.indexOfKey(fileId) >= 0
+	private fun hasFileId(fileId: Int): Boolean {
+		return files.indexOfKey(fileId) >= 0
+	}
 
 	fun canAccess(filePath: String?): Boolean {
 		return storageScopeIdentifier.canAccess(filePath)
@@ -121,7 +126,7 @@ class FileAccessHandler(val context: Context) {
 	 * Returns a positive (> 0) file id when the operation succeeds.
 	 * Otherwise, returns a negative value of [Error].
 	 */
-	fun fileOpen(path: String?, modeFlags: Int): Int {
+	fun fileOpen(path: String?, modeFlags: Int): Int = lock.withLock {
 		val (fileError, fileId) = fileOpen(path, FileAccessFlags.fromNativeModeFlags(modeFlags))
 		return if (fileError == Error.OK) {
 			fileId
@@ -145,7 +150,6 @@ class FileAccessHandler(val context: Context) {
 		return try {
 			path?.let {
 				val dataAccess = DataAccess.generateDataAccess(storageScope, context, it, accessFlag) ?: return FILE_OPEN_FAILED
-
 				files.put(++lastFileId, dataAccess)
 				Pair(Error.OK, lastFileId)
 			} ?: FILE_OPEN_FAILED
@@ -159,7 +163,7 @@ class FileAccessHandler(val context: Context) {
 		}
 	}
 
-	fun fileGetSize(fileId: Int): Long {
+	fun fileGetSize(fileId: Int): Long = lock.withLock {
 		if (!hasFileId(fileId)) {
 			return 0L
 		}
@@ -167,7 +171,7 @@ class FileAccessHandler(val context: Context) {
 		return files[fileId].size()
 	}
 
-	fun fileSeek(fileId: Int, position: Long) {
+	fun fileSeek(fileId: Int, position: Long) = lock.withLock {
 		if (!hasFileId(fileId)) {
 			return
 		}
@@ -175,7 +179,7 @@ class FileAccessHandler(val context: Context) {
 		files[fileId].seek(position)
 	}
 
-	fun fileSeekFromEnd(fileId: Int, position: Long) {
+	fun fileSeekFromEnd(fileId: Int, position: Long) = lock.withLock {
 		if (!hasFileId(fileId)) {
 			return
 		}
@@ -183,7 +187,7 @@ class FileAccessHandler(val context: Context) {
 		files[fileId].seekFromEnd(position)
 	}
 
-	fun fileRead(fileId: Int, byteBuffer: ByteBuffer?): Int {
+	fun fileRead(fileId: Int, byteBuffer: ByteBuffer?): Int = lock.withLock {
 		if (!hasFileId(fileId) || byteBuffer == null) {
 			return 0
 		}
@@ -191,7 +195,7 @@ class FileAccessHandler(val context: Context) {
 		return files[fileId].read(byteBuffer)
 	}
 
-	fun fileWrite(fileId: Int, byteBuffer: ByteBuffer?): Boolean {
+	fun fileWrite(fileId: Int, byteBuffer: ByteBuffer?): Boolean = lock.withLock {
 		if (!hasFileId(fileId) || byteBuffer == null) {
 			return false
 		}
@@ -199,7 +203,7 @@ class FileAccessHandler(val context: Context) {
 		return files[fileId].write(byteBuffer)
 	}
 
-	fun fileFlush(fileId: Int) {
+	fun fileFlush(fileId: Int) = lock.withLock {
 		if (!hasFileId(fileId)) {
 			return
 		}
@@ -243,7 +247,7 @@ class FileAccessHandler(val context: Context) {
 		}
 	}
 
-	fun fileResize(fileId: Int, length: Long): Int {
+	fun fileResize(fileId: Int, length: Long): Int = lock.withLock {
 		if (!hasFileId(fileId)) {
 			return Error.FAILED.toNativeValue()
 		}
@@ -266,7 +270,7 @@ class FileAccessHandler(val context: Context) {
 		}
 	}
 
-	fun fileGetPosition(fileId: Int): Long {
+	fun fileGetPosition(fileId: Int): Long = lock.withLock {
 		if (!hasFileId(fileId)) {
 			return 0L
 		}
@@ -274,7 +278,7 @@ class FileAccessHandler(val context: Context) {
 		return files[fileId].position()
 	}
 
-	fun isFileEof(fileId: Int): Boolean {
+	fun isFileEof(fileId: Int): Boolean = lock.withLock {
 		if (!hasFileId(fileId)) {
 			return false
 		}
@@ -282,12 +286,12 @@ class FileAccessHandler(val context: Context) {
 		return files[fileId].endOfFile
 	}
 
-	fun setFileEof(fileId: Int, eof: Boolean) {
+	fun setFileEof(fileId: Int, eof: Boolean) = lock.withLock {
 		val file = files[fileId] ?: return
 		file.endOfFile = eof
 	}
 
-	fun fileClose(fileId: Int) {
+	fun fileClose(fileId: Int) = lock.withLock {
 		if (hasFileId(fileId)) {
 			files[fileId].close()
 			files.remove(fileId)