Cherry-pick #4196 and #4204 to 1.7 (#4205)

azure-pipelines.yml

@@ -156,30 +156,31 @@ jobs:
       script: |
         $(PsExec.secureFilePath) -accepteula -s -i $(buildOutDir)\AppInstallerCLITests\AppInstallerCLITests.exe -logto $(artifactsDir)\AICLI-Unpackaged-System.log -s -r junit -o $(artifactsDir)\TEST-AppInstallerCLI-Unpackaged-System.xml
       workingDirectory: '$(buildOutDir)\AppInstallerCLITests'
-    continueOnError: true
+    condition: succeededOrFailed()
 
   - task: PowerShell@2
     displayName: Create Package Layout
     inputs:
       filePath: 'src\AppInstallerCLIPackage\Execute-AppxRecipe.ps1'
       arguments: '-AppxRecipePath AppInstallerCLIPackage\bin\$(buildPlatform)\$(buildConfiguration)\AppInstallerCLIPackage.build.appxrecipe -LayoutPath $(packageLayoutDir) -Force -Verbose'
       workingDirectory: 'src'
-    continueOnError: true
+    condition: succeededOrFailed()
 
   - task: PowerShell@2
     displayName: Run Unit Tests Packaged
     inputs:
       filePath: 'src\AppInstallerCLITests\Run-TestsInPackage.ps1'
       arguments: '-Args "~[pips]" -BuildRoot $(buildOutDir) -PackageRoot $(packageLayoutDir) -LogTarget $(artifactsDir)\AICLI-Packaged.log -TestResultsTarget $(artifactsDir)\TEST-AppInstallerCLI-Packaged.xml -ScriptWait'
       workingDirectory: 'src'
-    continueOnError: true
+    condition: succeededOrFailed()
 
   - task: PublishTestResults@2
     displayName: Publish Unit Test Results
     inputs:
       testResultsFormat: 'JUnit'
       testResultsFiles: '$(artifactsDir)\TEST-*.xml'
       failTaskOnFailedTests: true
+    condition: succeededOrFailed()
 
   - task: MSBuild@1
     displayName: Build MSIX Test Installer File
@@ -191,6 +192,7 @@ jobs:
                          /p:AppxBundle=Never
                          /p:UapAppxPackageBuildMode=SideLoadOnly
                          /p:AppxPackageSigningEnabled=false'
+    condition: succeededOrFailed()
 
   - task: PowerShell@2
     displayName: 'Set program files directory'
@@ -202,6 +204,7 @@ jobs:
         } else {
           Write-Host "##vso[task.setvariable variable=platformProgramFiles;]${env:ProgramFiles}"
         }
+    condition: succeededOrFailed()
 
   # Resolves resource strings utilized by InProc E2E tests.
   - task: CopyFiles@2
@@ -210,6 +213,7 @@ jobs:
       SourceFolder: '$(buildOutDir)\AppInstallerCLI'
       TargetFolder:  '$(platformProgramFiles)\dotnet'
       Contents: resources.pri
+    condition: succeededOrFailed()
 
   # Winmd accessed by test runner process (dotnet.exe)
   - task: CopyFiles@2
@@ -218,6 +222,7 @@ jobs:
       SourceFolder: '$(buildOutDir)\Microsoft.Management.Deployment'
       TargetFolder: '$(platformProgramFiles)\dotnet'
       Contents: Microsoft.Management.Deployment.winmd
+    condition: succeededOrFailed()
 
   - template: templates/e2e-setup.yml
     parameters:

src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw

@@ -2804,14 +2804,17 @@ Please specify one of them using the --source option to proceed.</value>
   <data name="RepairOperationNotSupported" xml:space="preserve">
     <value>The installer technology in use does not support repair.</value>
   </data>
-    <data name="NoAdminRepairForUserScopePackage" xml:space="preserve">
+  <data name="NoAdminRepairForUserScopePackage" xml:space="preserve">
     <value>The package installed for user scope cannot be repaired when running with administrator privileges.</value>
   </data>
   <data name="APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_REPAIR_PROHIBITED" xml:space="preserve">
     <value>Repair operations involving administrator privileges are not permitted on packages installed within the user scope.</value>
-  </data>  
+  </data>
   <data name="RepairFailedWithCode" xml:space="preserve">
     <value>Repair failed with exit code: {0}</value>
     <comment>{Locked="{0}"} Error message displayed when an attempt to repair an application package fails. {0} is a placeholder replaced by an error code.</comment>
   </data>
+  <data name="APPINSTALLER_CLI_ERROR_SQLITE_CONNECTION_TERMINATED" xml:space="preserve">
+    <value>The SQLite connection was terminated to prevent corruption.</value>
+  </data>
 </root>

src/AppInstallerCLITests/SQLiteWrapper.cpp

@@ -312,6 +312,90 @@ TEST_CASE("SQLiteWrapper_PrepareFailure", "[sqlitewrapper]")
     REQUIRE_THROWS_HR(builder.Prepare(connection), MAKE_HRESULT(SEVERITY_ERROR, FACILITY_SQLITE, SQLITE_ERROR));
 }
 
+TEST_CASE("SQLiteWrapper_BusyTimeout_None", "[sqlitewrapper]")
+{
+    TestCommon::TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
+    INFO("Using temporary file named: " << tempFile.GetPath());
+
+    wil::unique_event busy, done;
+    busy.create();
+    done.create();
+
+    std::thread busyThread([&]()
+        {
+            Connection threadConnection = Connection::Create(tempFile, Connection::OpenDisposition::Create);
+            Statement threadStatement = Statement::Create(threadConnection, "BEGIN EXCLUSIVE TRANSACTION");
+            threadStatement.Execute();
+            busy.SetEvent();
+            done.wait(500);
+        });
+    busyThread.detach();
+
+    busy.wait(500);
+
+    Connection testConnection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite);
+    testConnection.SetBusyTimeout(0ms);
+    Statement testStatement = Statement::Create(testConnection, "BEGIN EXCLUSIVE TRANSACTION");
+    REQUIRE_THROWS_HR(testStatement.Execute(), MAKE_HRESULT(SEVERITY_ERROR, FACILITY_SQLITE, SQLITE_BUSY));
+
+    done.SetEvent();
+}
+
+TEST_CASE("SQLiteWrapper_BusyTimeout_Some", "[sqlitewrapper]")
+{
+    TestCommon::TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
+    INFO("Using temporary file named: " << tempFile.GetPath());
+
+    wil::unique_event busy, ready, done;
+    busy.create();
+    ready.create();
+    done.create();
+
+    std::thread busyThread([&]()
+        {
+            Connection threadConnection = Connection::Create(tempFile, Connection::OpenDisposition::Create);
+            Statement threadBeginStatement = Statement::Create(threadConnection, "BEGIN EXCLUSIVE TRANSACTION");
+            Statement threadCommitStatement = Statement::Create(threadConnection, "COMMIT");
+            threadBeginStatement.Execute();
+            busy.SetEvent();
+            ready.wait(500);
+            done.wait(100);
+            threadCommitStatement.Execute();
+        });
+    busyThread.detach();
+
+    busy.wait(500);
+
+    Connection testConnection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite);
+    testConnection.SetBusyTimeout(500ms);
+    Statement testStatement = Statement::Create(testConnection, "BEGIN EXCLUSIVE TRANSACTION");
+    ready.SetEvent();
+    testStatement.Execute();
+
+    done.SetEvent();
+}
+
+TEST_CASE("SQLiteWrapper_CloseConnectionOnError", "[sqlitewrapper]")
+{
+    Connection connection = Connection::Create(SQLITE_MEMORY_DB_CONNECTION_TARGET, Connection::OpenDisposition::Create);
+
+    Builder::StatementBuilder builder;
+    builder.CreateTable(s_tableName).Columns({
+        Builder::ColumnBuilder(s_firstColumn, Builder::Type::Int),
+        Builder::ColumnBuilder(s_secondColumn, Builder::Type::Text),
+        });
+
+    Statement createTable = builder.Prepare(connection);
+    REQUIRE_FALSE(createTable.Step());
+    REQUIRE(createTable.GetState() == Statement::State::Completed);
+
+    createTable.Reset();
+    REQUIRE_THROWS(createTable.Step(true));
+
+    // Do anything that needs the connection
+    REQUIRE_THROWS_HR(connection.GetLastInsertRowID(), APPINSTALLER_CLI_ERROR_SQLITE_CONNECTION_TERMINATED);
+}
+
 TEST_CASE("SQLBuilder_SimpleSelectBind", "[sqlbuilder]")
 {
     Connection connection = Connection::Create(SQLITE_MEMORY_DB_CONNECTION_TARGET, Connection::OpenDisposition::Create);

src/AppInstallerCLITests/TestCommon.cpp

@@ -85,7 +85,7 @@ namespace TestCommon
         }
     }
 
-    TempFile::~TempFile()
+    TempFile::~TempFile() try
     {
         switch (s_TempFileDestructorBehavior)
         {
@@ -99,6 +99,7 @@ namespace TestCommon
             break;
         }
     }
+    CATCH_LOG_RETURN()
 
     void TempFile::Rename(const std::filesystem::path& newFilePath)
     {

src/AppInstallerSharedLib/Errors.cpp

@@ -213,6 +213,7 @@ namespace AppInstaller
             WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_EXEC_REPAIR_FAILED, "Repair operation failed."),
             WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_REPAIR_NOT_SUPPORTED, "The installer technology in use doesn't support repair."),
             WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_REPAIR_PROHIBITED, "Repair operations involving administrator privileges are not permitted on packages installed within the user scope."),
+            WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_SQLITE_CONNECTION_TERMINATED, "The SQLite connection was terminated to prevent corruption."),
 
             // Install errors.
             WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE, "Application is currently running. Exit the application then try again."),

src/AppInstallerSharedLib/Public/AppInstallerErrors.h

@@ -143,6 +143,7 @@
 #define APPINSTALLER_CLI_ERROR_EXEC_REPAIR_FAILED                   ((HRESULT)0x8A15007B)
 #define APPINSTALLER_CLI_ERROR_REPAIR_NOT_SUPPORTED                 ((HRESULT)0x8A15007C)
 #define APPINSTALLER_CLI_ERROR_ADMIN_CONTEXT_REPAIR_PROHIBITED      ((HRESULT)0x8A15007D)
+#define APPINSTALLER_CLI_ERROR_SQLITE_CONNECTION_TERMINATED         ((HRESULT)0x8A15007E)
 
 // Install errors.
 #define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE                   ((HRESULT)0x8A150101)

src/AppInstallerSharedLib/Public/winget/SQLiteWrapper.h

@@ -8,6 +8,7 @@
 #include <AppInstallerLogging.h>
 #include <AppInstallerLanguageUtilities.h>
 
+#include <memory>
 #include <string>
 #include <string_view>
 #include <tuple>
@@ -125,6 +126,23 @@ namespace AppInstaller::SQLite
 
         template <typename T>
         using ParameterSpecifics = ParameterSpecificsImpl<std::decay_t<T>>;
+
+        // Allows the connection to be shared so that it can be closed in some circumstances.
+        struct SharedConnection
+        {
+            // Disables the connection, causing an exception to be thrown by `get`.
+            void Disable();
+
+            // Gets the connection object if active.
+            sqlite3* Get() const;
+
+            // Gets the connection object for creation.
+            sqlite3** GetPtr();
+
+        private:
+            std::atomic_bool m_active = true;
+            wil::unique_any<sqlite3*, decltype(sqlite3_close_v2), sqlite3_close_v2> m_dbconn;
+        };
     }
 
     // A SQLite exception.
@@ -133,9 +151,13 @@ namespace AppInstaller::SQLite
         SQLiteException(int error) : wil::ResultException(MAKE_HRESULT(SEVERITY_ERROR, FACILITY_SQLITE, error)) {}
     };
 
+    struct Statement;
+
     // The connection to a database.
     struct Connection
     {
+        friend Statement;
+
         // The disposition for opening a database connection.
         enum class OpenDisposition : int
         {
@@ -180,13 +202,20 @@ namespace AppInstaller::SQLite
         //. Gets the (fixed but arbitrary) identifier for this connection.
         size_t GetID() const;
 
-        operator sqlite3* () const { return m_dbconn.get(); }
+        // Sets the busy timeout for the connection.
+        void SetBusyTimeout(std::chrono::milliseconds timeout);
+
+        operator sqlite3* () const { return m_dbconn->Get(); }
+
+    protected:
+        // Gets the shared connection.
+        std::shared_ptr<details::SharedConnection> GetSharedConnection() const;
 
     private:
         Connection(const std::string& target, OpenDisposition disposition, OpenFlags flags);
 
         size_t m_id = 0;
-        wil::unique_any<sqlite3*, decltype(sqlite3_close_v2), sqlite3_close_v2> m_dbconn;
+        std::shared_ptr<details::SharedConnection> m_dbconn;
     };
 
     // A SQL statement.
@@ -234,10 +263,10 @@ namespace AppInstaller::SQLite
         // Evaluate the statement; either retrieving the next row or executing some action.
         // Returns true if there is a row of data, or false if there is none.
         // This return value is the equivalent of 'GetState() == State::HasRow' after calling Step.
-        bool Step(bool failFastOnError = false);
+        bool Step(bool closeConnectionOnError = false);
 
         // Equivalent to Step, but does not ever expect a result, throwing if one is retrieved.
-        void Execute(bool failFastOnError = false);
+        void Execute(bool closeConnectionOnError = false);
 
         // Gets a boolean value that indicates whether the specified column value is null in the current row.
         // The index is 0 based.
@@ -282,6 +311,7 @@ namespace AppInstaller::SQLite
             return std::make_tuple(details::ParameterSpecifics<Values>::GetColumn(m_stmt.get(), I)...);
         }
 
+        std::shared_ptr<details::SharedConnection> m_dbconn;
         size_t m_connectionId = 0;
         size_t m_id = 0;
         wil::unique_any<sqlite3_stmt*, decltype(sqlite3_finalize), sqlite3_finalize> m_stmt;
@@ -303,7 +333,7 @@ namespace AppInstaller::SQLite
         ~Savepoint();
 
         // Rolls back the Savepoint.
-        void Rollback();
+        void Rollback(bool throwOnError = true);
 
         // Commits the Savepoint.
         void Commit();