diff --git a/bindings/java/src/org/sleuthkit/datamodel/CaseDbAccessManager.java b/bindings/java/src/org/sleuthkit/datamodel/CaseDbAccessManager.java index e80c280aad430bd7124c8fbb9322189f72427250..1bf72c77351b042721c2fa4cef32807d6d4352d6 100644 --- a/bindings/java/src/org/sleuthkit/datamodel/CaseDbAccessManager.java +++ b/bindings/java/src/org/sleuthkit/datamodel/CaseDbAccessManager.java @@ -307,7 +307,6 @@ public void alterTable(final String tableName, final String alterSQL, final Case validateSQL(alterSQL); CaseDbConnection connection = transaction.getConnection(); - transaction.acquireSingleUserCaseWriteLock(); Statement statement = null; String sql = "ALTER TABLE " + tableName + " " + alterSQL; @@ -328,7 +327,6 @@ public void alterTable(final String tableName, final String alterSQL, final Case throw new TskCoreException(String.format("Error altering table %s with SQL = %s", tableName, sql), ex); } finally { closeStatement(statement); - // NOTE: write lock will be released by transaction } } @@ -422,7 +420,6 @@ public long insert(final String tableName, final String sql, final CaseDbTransac validateSQL(sql); CaseDbConnection connection = transaction.getConnection(); - transaction.acquireSingleUserCaseWriteLock(); PreparedStatement statement = null; ResultSet resultSet; @@ -444,7 +441,6 @@ public long insert(final String tableName, final String sql, final CaseDbTransac throw new TskCoreException("Error inserting row in table " + tableName + " with sql = "+ insertSQL, ex); } finally { closeStatement(statement); - // NOTE: write lock will be released by transaction } return rowId; @@ -507,7 +503,6 @@ public long insertOrUpdate(final String tableName, final String sql, final CaseD validateSQL(sql); CaseDbConnection connection = transaction.getConnection(); - transaction.acquireSingleUserCaseWriteLock(); PreparedStatement statement = null; ResultSet resultSet; @@ -528,7 +523,6 @@ public long insertOrUpdate(final String tableName, final String sql, final CaseD throw new TskCoreException("Error inserting row in table " + tableName + " with sql = "+ insertSQL, ex); } finally { closeStatement(statement); - // NOTE: write lock will be released by transaction } return rowId; @@ -576,7 +570,6 @@ public void update(final String tableName, final String sql, CaseDbTransaction t validateSQL(sql); CaseDbConnection connection = transaction.getConnection(); - transaction.acquireSingleUserCaseWriteLock(); Statement statement = null; String updateSQL = "UPDATE " + tableName + " " + sql; // NON-NLS @@ -588,7 +581,6 @@ public void update(final String tableName, final String sql, CaseDbTransaction t throw new TskCoreException("Error Updating table " + tableName, ex); } finally { closeStatement(statement); - // NOTE: write lock will be released by transaction } } diff --git a/bindings/java/src/org/sleuthkit/datamodel/JniDbHelper.java b/bindings/java/src/org/sleuthkit/datamodel/JniDbHelper.java index b65ad77b136adda0578a7bce53ed9b2f56945902..4476402935fdc0e9c1f41b9d1e3fdbb6bfd1143e 100644 --- a/bindings/java/src/org/sleuthkit/datamodel/JniDbHelper.java +++ b/bindings/java/src/org/sleuthkit/datamodel/JniDbHelper.java @@ -61,7 +61,6 @@ class JniDbHelper { */ private void beginTransaction() throws TskCoreException { trans = caseDb.beginTransaction(); - trans.acquireSingleUserCaseWriteLock(); } /** diff --git a/bindings/java/src/org/sleuthkit/datamodel/SleuthkitCase.java b/bindings/java/src/org/sleuthkit/datamodel/SleuthkitCase.java index 2145d522629396322718bf6c02870479fb8f04e2..12d121f1b98a17189b8984e3d7841304a401897b 100644 --- a/bindings/java/src/org/sleuthkit/datamodel/SleuthkitCase.java +++ b/bindings/java/src/org/sleuthkit/datamodel/SleuthkitCase.java @@ -2291,6 +2291,10 @@ public String getBackupDatabasePath() { * that is returned can be passed to methods that take a CaseDbTransaction. * The caller is responsible for calling either commit() or rollback() on * the transaction object. + * + * Note that this beginning the transaction also acquires the single user + * case write lock, which will be automatically released when the transaction + * is closed. * * @return A CaseDbTransaction object. * @@ -5458,14 +5462,12 @@ public List<AbstractFile> findFiles(Content dataSource, String fileName, String */ public VirtualDirectory addVirtualDirectory(long parentId, String directoryName) throws TskCoreException { CaseDbTransaction localTrans = beginTransaction(); - localTrans.acquireSingleUserCaseWriteLock(); try { VirtualDirectory newVD = addVirtualDirectory(parentId, directoryName, localTrans); localTrans.commit(); localTrans = null; return newVD; } finally { - // NOTE: write lock will be released by transaction if (null != localTrans) { try { localTrans.rollback(); @@ -5540,7 +5542,6 @@ public VirtualDirectory addVirtualDirectory(long parentId, String directoryName, throw new TskCoreException("Passed null CaseDbTransaction"); } - transaction.acquireSingleUserCaseWriteLock(); ResultSet resultSet = null; try { // Get the parent path. @@ -5638,7 +5639,6 @@ public VirtualDirectory addVirtualDirectory(long parentId, String directoryName, throw new TskCoreException("Error creating virtual directory '" + directoryName + "'", e); } finally { closeResultSet(resultSet); - // NOTE: write lock will be released by transaction } } @@ -5655,7 +5655,6 @@ public VirtualDirectory addVirtualDirectory(long parentId, String directoryName, * @throws TskCoreException */ public LocalDirectory addLocalDirectory(long parentId, String directoryName) throws TskCoreException { - acquireSingleUserCaseWriteLock(); CaseDbTransaction localTrans = beginTransaction(); try { LocalDirectory newLD = addLocalDirectory(parentId, directoryName, localTrans); @@ -5668,8 +5667,6 @@ public LocalDirectory addLocalDirectory(long parentId, String directoryName) thr logger.log(Level.SEVERE, String.format("Failed to rollback transaction after exception: %s", ex.getMessage()), ex2); } throw ex; - } finally { - releaseSingleUserCaseWriteLock(); } } @@ -5695,7 +5692,6 @@ public LocalDirectory addLocalDirectory(long parentId, String directoryName, Cas throw new TskCoreException("Passed null CaseDbTransaction"); } - transaction.acquireSingleUserCaseWriteLock(); ResultSet resultSet = null; try { // Get the parent path. @@ -5774,7 +5770,6 @@ public LocalDirectory addLocalDirectory(long parentId, String directoryName, Cas throw new TskCoreException("Error creating local directory '" + directoryName + "'", e); } finally { closeResultSet(resultSet); - // NOTE: write lock will be released by transaction } } @@ -6166,7 +6161,6 @@ public FsContent addFileSystemFile(long dataSourceObjId, long fsObjId, Statement queryStatement = null; try { CaseDbConnection connection = transaction.getConnection(); - transaction.acquireSingleUserCaseWriteLock(); // Insert a row for the local/logical file into the tsk_objects table. // INSERT INTO tsk_objects (par_obj_id, type) VALUES (?, ?) @@ -6307,7 +6301,6 @@ public final List<LayoutFile> addLayoutFiles(Content parent, List<TskFileRange> try { transaction = beginTransaction(); - transaction.acquireSingleUserCaseWriteLock(); CaseDbConnection connection = transaction.getConnection(); List<LayoutFile> fileRangeLayoutFiles = new ArrayList<LayoutFile>(); @@ -6396,7 +6389,6 @@ public final List<LayoutFile> addLayoutFiles(Content parent, List<TskFileRange> closeResultSet(resultSet); closeStatement(statement); - // NOTE: write lock will be released by transaction if (null != transaction) { try { transaction.rollback(); @@ -6437,7 +6429,6 @@ public final List<LayoutFile> addCarvedFiles(CarvingResult carvingResult) throws long newCacheKey = 0; // Used to roll back cache if transaction is rolled back. try { transaction = beginTransaction(); - transaction.acquireSingleUserCaseWriteLock(); CaseDbConnection connection = transaction.getConnection(); /* @@ -6582,7 +6573,6 @@ public final List<LayoutFile> addCarvedFiles(CarvingResult carvingResult) throws closeResultSet(resultSet); closeStatement(statement); - // NOTE: write lock will be released by transaction if (null != transaction) { try { transaction.rollback(); @@ -6634,7 +6624,6 @@ public DerivedFile addDerivedFile(String fileName, String localPath, // Strip off any leading slashes from the local path (leading slashes indicate absolute paths) localPath = localPath.replaceAll("^[/\\\\]+", ""); - acquireSingleUserCaseWriteLock(); TimelineManager timelineManager = getTimelineManager(); CaseDbTransaction transaction = beginTransaction(); @@ -6730,7 +6719,6 @@ public DerivedFile addDerivedFile(String fileName, String localPath, throw new TskCoreException("Failed to add derived file to case database", ex); } finally { connection.close(); - releaseSingleUserCaseWriteLock(); } } @@ -6951,7 +6939,6 @@ public LocalFile addLocalFile(String fileName, String localPath, boolean isFile, TskData.EncodingType encodingType, Content parent, CaseDbTransaction transaction) throws TskCoreException { CaseDbConnection connection = transaction.getConnection(); - transaction.acquireSingleUserCaseWriteLock(); Statement queryStatement = null; try { @@ -7037,7 +7024,6 @@ public LocalFile addLocalFile(String fileName, String localPath, throw new TskCoreException(String.format("Failed to INSERT local file %s (%s) with parent id %d in tsk_files table", fileName, localPath, parent.getId()), ex); } finally { closeStatement(queryStatement); - // NOTE: write lock will be released by transaction } } @@ -7055,7 +7041,6 @@ public LocalFile addLocalFile(String fileName, String localPath, */ private boolean isRootDirectory(AbstractFile file, CaseDbTransaction transaction) throws TskCoreException { CaseDbConnection connection = transaction.getConnection(); - transaction.acquireSingleUserCaseWriteLock(); Statement statement = null; ResultSet resultSet = null; @@ -7085,7 +7070,6 @@ private boolean isRootDirectory(AbstractFile file, CaseDbTransaction transaction } finally { closeResultSet(resultSet); closeStatement(statement); - // NOTE: write lock will be released by transaction } } @@ -7131,7 +7115,6 @@ public LayoutFile addLayoutFile(String fileName, ResultSet resultSet = null; try { transaction = beginTransaction(); - transaction.acquireSingleUserCaseWriteLock(); CaseDbConnection connection = transaction.getConnection(); /* @@ -7230,7 +7213,6 @@ public LayoutFile addLayoutFile(String fileName, closeResultSet(resultSet); closeStatement(statement); - // NOTE: write lock will be released by transaction if (null != transaction) { try { transaction.rollback(); @@ -9757,10 +9739,12 @@ public TagName addOrUpdateTagName(String displayName, String description, TagNam long tagId = resultSet.getLong(1); + resultSet.close(); statement = connection.getPreparedStatement(PREPARED_STATEMENT.SELECT_TAG_NAME_BY_ID); statement.clearParameters(); statement.setLong(1, tagId); resultSet = connection.executeQuery(statement); + resultSet.next(); return new TagName(this, tagId, displayName, description, color, knownStatus, resultSet.getLong("tag_set_id"), resultSet.getInt("rank")); @@ -12102,11 +12086,16 @@ void executeCommand(DbCommand command) throws SQLException { * Transaction interface because that sort of flexibility and its associated * complexity is not needed. Also, TskCoreExceptions are thrown to be * consistent with the outer SleuthkitCase class. + * + * This class will automatically acquire the single user case write lock + * and release it when the transaction is closed. Otherwise we risk deadlock + * because this transaction can lock up SQLite and make it "busy" and + * another thread may get a write lock to the DB, but not + * be able to do anything because the DB is busy. */ public static final class CaseDbTransaction { private final CaseDbConnection connection; - private boolean hasWriteLock = false; private SleuthkitCase sleuthkitCase; private CaseDbTransaction(SleuthkitCase sleuthkitCase, CaseDbConnection connection) throws TskCoreException { @@ -12117,6 +12106,7 @@ private CaseDbTransaction(SleuthkitCase sleuthkitCase, CaseDbConnection connecti } catch (SQLException ex) { throw new TskCoreException("Failed to create transaction on case database", ex); } + sleuthkitCase.acquireSingleUserCaseWriteLock(); } /** @@ -12130,23 +12120,6 @@ CaseDbConnection getConnection() { return this.connection; } - /** - * Obtain a write lock for this transaction. Only one will be obtained - * (no matter how many times it is called) and will be released when - * commit or rollback is called. - * - * If this is not used, you risk deadlock because this transaction can - * lock up SQLite and make it "busy" and another thread may get a write - * lock to the DB, but not be able to do anything because the DB is - * busy. - */ - void acquireSingleUserCaseWriteLock() { - if (!hasWriteLock) { - hasWriteLock = true; - sleuthkitCase.acquireSingleUserCaseWriteLock(); - } - } - /** * Commits the transaction on the case database that was begun when this * object was constructed. @@ -12185,10 +12158,7 @@ public void rollback() throws TskCoreException { */ void close() { this.connection.close(); - if (hasWriteLock) { - sleuthkitCase.releaseSingleUserCaseWriteLock(); - hasWriteLock = false; - } + sleuthkitCase.releaseSingleUserCaseWriteLock(); } } diff --git a/bindings/java/test/org/sleuthkit/datamodel/CommunicationsManagerTest.java b/bindings/java/test/org/sleuthkit/datamodel/CommunicationsManagerTest.java index d71ddd4e35e66e5ed5da6c5fce5d2ba33730b6b1..d2e025b95d3fd6a6b0cc8baf5afe6cf0874934b2 100644 --- a/bindings/java/test/org/sleuthkit/datamodel/CommunicationsManagerTest.java +++ b/bindings/java/test/org/sleuthkit/datamodel/CommunicationsManagerTest.java @@ -253,7 +253,6 @@ public static void setUpClass() { System.out.println("CommsMgr Test DB created at: " + dbPath); SleuthkitCase.CaseDbTransaction trans = caseDB.beginTransaction(); - trans.acquireSingleUserCaseWriteLock(); LocalFilesDataSource dataSource_1 = caseDB.addLocalFilesDataSource(DS1_DEVICEID, ROOTDIR_1, "", trans); LocalFilesDataSource dataSource_2 = caseDB.addLocalFilesDataSource(DS2_DEVICEID, ROOTDIR_2, "", trans);