Skip to content

Commit efa74f9

Browse files
JohnMcPMSJohnMcPMS
committed
PR feedback, test fixes, integrate CLI with shutdown to prevent premature termination
1 parent 51309d3 commit efa74f9

File tree

9 files changed

+66
-33
lines changed

9 files changed

+66
-33
lines changed

‎src/AppInstallerCLI.sln

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -972,30 +972,24 @@ Global
972972
{9406322E-6272-487E-902A-9953889719EA}.ReleaseStatic|x64.Build.0 = ReleaseStatic|Any CPU
973973
{9406322E-6272-487E-902A-9953889719EA}.ReleaseStatic|x86.ActiveCfg = ReleaseStatic|Any CPU
974974
{9406322E-6272-487E-902A-9953889719EA}.ReleaseStatic|x86.Build.0 = ReleaseStatic|Any CPU
975-
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Debug|ARM64.ActiveCfg = Debug|x64
976-
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Debug|ARM64.Build.0 = Debug|x64
975+
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Debug|ARM64.ActiveCfg = Debug|arm64
976+
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Debug|ARM64.Build.0 = Debug|arm64
977977
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Debug|x64.ActiveCfg = Debug|x64
978978
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Debug|x64.Build.0 = Debug|x64
979979
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Debug|x86.ActiveCfg = Debug|x86
980980
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Debug|x86.Build.0 = Debug|x86
981-
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Fuzzing|ARM64.ActiveCfg = Release|x64
982-
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Fuzzing|ARM64.Build.0 = Release|x64
981+
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Fuzzing|ARM64.ActiveCfg = Release|arm64
983982
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Fuzzing|x64.ActiveCfg = Release|x64
984-
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Fuzzing|x64.Build.0 = Release|x64
985983
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Fuzzing|x86.ActiveCfg = Release|x86
986-
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Fuzzing|x86.Build.0 = Release|x86
987-
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Release|ARM64.ActiveCfg = Release|x64
988-
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Release|ARM64.Build.0 = Release|x64
984+
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Release|ARM64.ActiveCfg = Release|arm64
985+
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Release|ARM64.Build.0 = Release|arm64
989986
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Release|x64.ActiveCfg = Release|x64
990987
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Release|x64.Build.0 = Release|x64
991988
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Release|x86.ActiveCfg = Release|x86
992989
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.Release|x86.Build.0 = Release|x86
993-
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.ReleaseStatic|ARM64.ActiveCfg = Release|x64
994-
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.ReleaseStatic|ARM64.Build.0 = Release|x64
990+
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.ReleaseStatic|ARM64.ActiveCfg = Release|arm64
995991
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.ReleaseStatic|x64.ActiveCfg = Release|x64
996-
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.ReleaseStatic|x64.Build.0 = Release|x64
997992
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.ReleaseStatic|x86.ActiveCfg = Release|x86
998-
{7139ED6E-8FBC-0B61-3E3A-AA2A23CC4D6A}.ReleaseStatic|x86.Build.0 = Release|x86
999993
{33745E4A-39E2-676F-7E23-50FB43848D25}.Debug|ARM64.ActiveCfg = Debug|arm64
1000994
{33745E4A-39E2-676F-7E23-50FB43848D25}.Debug|ARM64.Build.0 = Debug|arm64
1001995
{33745E4A-39E2-676F-7E23-50FB43848D25}.Debug|x64.ActiveCfg = Debug|x64

‎src/AppInstallerCLICore/Commands/TestCommand.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ namespace AppInstaller::CLI
2020
{
2121
namespace
2222
{
23-
void LogAndReport(Execution::Context& context, const std::string& message)
23+
void LogAndReport(Execution::Context& context, std::string_view message)
2424
{
25-
AICLI_LOG(CLI, Info, << message);
2625
context.Reporter.Info() << message << std::endl;
26+
AICLI_LOG(CLI, Info, << message);
2727
}
2828

2929
HRESULT WaitForShutdown(Execution::Context& context)

‎src/AppInstallerCLICore/ContextOrchestrator.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ namespace AppInstaller::CLI::Execution
8787
return {};
8888
}
8989

90-
void ContextOrchestrator::EnqueueAndRunItem(std::shared_ptr<OrchestratorQueueItem> item)
90+
void ContextOrchestrator::EnqueueAndRunItem(const std::shared_ptr<OrchestratorQueueItem>& item)
9191
{
9292
std::lock_guard<std::mutex> lockQueue{ m_queueLock };
9393

@@ -265,7 +265,7 @@ namespace AppInstaller::CLI::Execution
265265
return {};
266266
}
267267

268-
void OrchestratorQueue::EnqueueItem(std::shared_ptr<OrchestratorQueueItem> item)
268+
void OrchestratorQueue::EnqueueItem(const std::shared_ptr<OrchestratorQueueItem>& item)
269269
{
270270
{
271271
std::lock_guard<std::mutex> lockQueue{ m_itemLock };
@@ -324,7 +324,7 @@ namespace AppInstaller::CLI::Execution
324324
CloseThreadpoolCleanupGroupMembers(m_threadPoolCleanupGroup.get(), false, nullptr);
325325
}
326326

327-
void OrchestratorQueue::EnqueueAndRunItem(std::shared_ptr<OrchestratorQueueItem> item)
327+
void OrchestratorQueue::EnqueueAndRunItem(const std::shared_ptr<OrchestratorQueueItem>& item)
328328
{
329329
EnqueueItem(item);
330330

‎src/AppInstallerCLICore/ContextOrchestrator.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ namespace AppInstaller::CLI::Execution
119119
ContextOrchestrator(unsigned int hardwareConcurrency);
120120
static ContextOrchestrator& Instance();
121121

122-
void EnqueueAndRunItem(std::shared_ptr<OrchestratorQueueItem> queueItem);
122+
void EnqueueAndRunItem(const std::shared_ptr<OrchestratorQueueItem>& queueItem);
123123
void CancelQueueItem(const OrchestratorQueueItem& item);
124124

125125
std::shared_ptr<OrchestratorQueueItem> GetQueueItem(const OrchestratorQueueItemId& queueItemId);
@@ -139,7 +139,6 @@ namespace AppInstaller::CLI::Execution
139139

140140
// Waits for running items to complete; waits up to full time out in *each* queue.
141141
// Returns true to indicate all queues are empty before the timeout.
142-
// Intended for test use.
143142
bool WaitForRunningItems(DWORD timeoutMilliseconds);
144143

145144
// Gets a string that represents the current state of the orchestrator.
@@ -171,7 +170,7 @@ namespace AppInstaller::CLI::Execution
171170
std::string_view CommandName() const { return m_commandName; }
172171

173172
// Enqueues an item to be run when there are threads available.
174-
void EnqueueAndRunItem(std::shared_ptr<OrchestratorQueueItem> item);
173+
void EnqueueAndRunItem(const std::shared_ptr<OrchestratorQueueItem>& item);
175174

176175
// Removes an item by id, provided that it is in the given state.
177176
// Returns true if an item was removed.
@@ -189,20 +188,18 @@ namespace AppInstaller::CLI::Execution
189188
void CancelAllItems(CancelReason reason);
190189

191190
// Waits until the empty queue event is signaled.
192-
// Intended for use during shutdown handling.
193191
void WaitForEmptyQueue();
194192

195193
// Waits until the empty queue event is signaled.
196194
// Returns true to indicate the queue is empty before the timeout.
197-
// Intended for tests.
198195
bool WaitForEmptyQueue(DWORD timeoutMilliseconds);
199196

200197
// Gets a string that represents the current state of the queue.
201198
std::string GetStatusString();
202199

203200
private:
204201
// Enqueues an item.
205-
void EnqueueItem(std::shared_ptr<OrchestratorQueueItem> item);
202+
void EnqueueItem(const std::shared_ptr<OrchestratorQueueItem>& item);
206203

207204
_Requires_lock_held_(m_itemLock)
208205
std::deque<std::shared_ptr<OrchestratorQueueItem>>::iterator FindIteratorById(const OrchestratorQueueItemId& comparisonQueueItemId);

‎src/AppInstallerCLICore/Core.cpp

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99
#include "Commands/InstallCommand.h"
1010
#include "COMContext.h"
1111
#include <AppInstallerFileLogger.h>
12-
#include <winget/OutputDebugStringLogger.h>
12+
#include <winget/OutputDebugStringLogger.h>
13+
#include "Public/ShutdownMonitoring.h"
1314

1415
#ifndef AICLI_DISABLE_TEST_HOOKS
1516
#include <winget/Debugging.h>
@@ -58,11 +59,33 @@ namespace AppInstaller::CLI
5859
#endif
5960

6061
std::_Exit(APPINSTALLER_CLI_ERROR_INTERNAL_ERROR);
62+
}
63+
64+
wil::slim_event_manual_reset& GetMainWaitEvent()
65+
{
66+
static wil::slim_event_manual_reset s_mainWait;
67+
return s_mainWait;
68+
}
69+
70+
void WaitOnMainWaitEvent()
71+
{
72+
GetMainWaitEvent().wait(5000);
73+
}
74+
75+
void RegisterShutdownBlocker()
76+
{
77+
ShutdownMonitoring::ServerShutdownSynchronization::ComponentSystem main{};
78+
main.Wait = WaitOnMainWaitEvent;
79+
ShutdownMonitoring::ServerShutdownSynchronization::AddComponent(main);
6180
}
6281
}
6382

6483
int CoreMain(int argc, wchar_t const** argv) try
6584
{
85+
// This prevents the OS package management from terminating the CLI process before it has had a chance to gracefully exit.
86+
RegisterShutdownBlocker();
87+
auto signalMainExit = wil::scope_exit([]() { GetMainWaitEvent().SetEvent(); });
88+
6689
std::signal(SIGABRT, abort_signal_handler);
6790

6891
init_apartment();

‎src/AppInstallerCLICore/ShutdownMonitoring.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,13 @@ namespace AppInstaller::ShutdownMonitoring
9696

9797
void TerminationSignalHandler::StartAppShutdown()
9898
{
99-
// Lifetime manager sends CTRL-C after the WM_QUERYENDSESSION is processed.
100-
// If we disable the CTRL-C handler, the default handler will kill us.
101-
InformListeners(CancelReason::AppShutdown, true);
102-
10399
#ifndef AICLI_DISABLE_TEST_HOOKS
104100
m_appShutdownEvent.SetEvent();
105101
#endif
102+
103+
// Lifetime manager sends CTRL-C after the WM_QUERYENDSESSION is processed.
104+
// If we disable the CTRL-C handler, the default handler will kill us.
105+
InformListeners(CancelReason::AppShutdown, true);
106106
}
107107

108108
BOOL WINAPI TerminationSignalHandler::StaticCtrlHandlerFunction(DWORD ctrlType)

‎src/AppInstallerCLIE2ETests/Interop/Shutdown.cs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,28 @@ public async Task ActiveInstallOperation()
8686

8787
Assert.IsTrue(server.Process.WaitForExit(5000));
8888

89-
var installResult = await installOperation;
89+
InstallResult installResult = null;
90+
Exception exception = null;
91+
92+
try
93+
{
94+
installResult = await installOperation;
95+
}
96+
catch (Exception ex)
97+
{
98+
exception = ex;
99+
}
100+
101+
// We just expect some kind of signal to indicate the failed attempt.
102+
if (installResult != null)
103+
{
104+
Assert.AreNotEqual(InstallResultStatus.Ok, installResult.Status);
105+
}
106+
else
107+
{
108+
Assert.NotNull(exception);
109+
}
90110

91-
Assert.AreNotEqual(InstallResultStatus.Ok, installResult.Status);
92111
Assert.False(TestCommon.VerifyTestExeInstalledAndCleanup(installDir));
93112
}
94113

‎src/Microsoft.Management.Configuration.UnitTests/Tests/ShutdownTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public void ShutdownSynchronization_SyncCall()
105105
Assert.NotNull(exception);
106106
Assert.IsType<OperationCanceledException>(exception);
107107

108-
Assert.True(server.Process.HasExited);
108+
Assert.True(server.Process.WaitForExit(5000));
109109
}
110110

111111
/// <summary>
@@ -160,7 +160,7 @@ public void ShutdownSynchronization_AsyncCall()
160160

161161
Assert.ThrowsAny<Exception>(() => operation.GetAwaiter().GetResult());
162162

163-
Assert.True(server.Process.HasExited);
163+
Assert.True(server.Process.WaitForExit(5000));
164164
}
165165

166166
private void SendMessageAndLog(WinGetServerInstance server, WindowMessage message)

‎src/WinGetTestCommon/WinGetTestCommon.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<PropertyGroup>
44
<TargetFramework>net8.0-windows</TargetFramework>
55
<OutDir>$(SolutionDir)$(Platform)\$(Configuration)\WinGetTestCommon\</OutDir>
6-
<Platforms>x64;x86</Platforms>
6+
<Platforms>x64;x86;arm64</Platforms>
77
<OutputType>Library</OutputType>
88
<Nullable>enable</Nullable>
99
</PropertyGroup>

0 commit comments

Comments
 (0)