Skip to content

Commit

Permalink
Enforce single install across winget processes (#3118)
Browse files Browse the repository at this point in the history
This change uses a named mutex to enforce a single install/uninstall operation is occurring for the entire session.  The current behavior is that every process was able to install a single thing at a time, but as we have more usage this could lead to a potential issue.

The mutex is acquired and held for the workflow task that routes out to the installer type specific tasks.  Some installer types are explicitly excluded, as they are known to contain their own synchronization internally.

In addition to the mutex, the ability to set a message to show with the indefinite progress (spinner) was added.  This allows for the message to be removed when the wait is over.  The message was not implemented into the progress bar, nor is it being sent to COM progress.

The COM scheduling was not made aware of the mutex, and so it will execute as it has in the past, blocking on the mutex when appropriate.  This could be improved in the future to enable the parallel capable installer types to operate.  One potential option would be a second installer processing thread that only operated on the excluded types, and thus would never block.
  • Loading branch information
JohnMcPMS authored Mar 31, 2023
1 parent dddc094 commit 741244e
Show file tree
Hide file tree
Showing 24 changed files with 753 additions and 434 deletions.
1 change: 1 addition & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ corecrt
count'th
countof
countryregion
CPIL
cplusplus
createmanifestmetadata
cswinrt
Expand Down
5 changes: 5 additions & 0 deletions src/AppInstallerCLICore/COMContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ namespace AppInstaller::CLI::Execution
FireCallbacks(ReportType::Progressing, current, maximum, progressType, m_executionStage);
}

void COMContext::SetProgressMessage(std::string_view)
{
// TODO: Consider sending message to COM progress
}

void COMContext::EndProgress(bool)
{
FireCallbacks(ReportType::EndProgress, 0, 0, ProgressType::None, m_executionStage);
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/COMContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ namespace AppInstaller::CLI::Execution
// IProgressSink
void BeginProgress() override;
void OnProgress(uint64_t current, uint64_t maximum, ProgressType type) override;
void SetProgressMessage(std::string_view message) override;
void EndProgress(bool) override;

//Execution::Context
Expand Down
13 changes: 13 additions & 0 deletions src/AppInstallerCLICore/ChannelStreams.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,19 @@ namespace AppInstaller::CLI::Execution
using namespace Settings;
using namespace VirtualTerminal;

size_t GetConsoleWidth()
{
CONSOLE_SCREEN_BUFFER_INFO consoleInfo{};
if (GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &consoleInfo))
{
return static_cast<size_t>(consoleInfo.dwSize.X);
}
else
{
return 120;
}
}

BaseStream::BaseStream(std::ostream& out, bool enabled, bool VTEnabled) :
m_out(out), m_enabled(enabled), m_VTEnabled(VTEnabled) {}

Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerCLICore/ChannelStreams.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@

namespace AppInstaller::CLI::Execution
{
// Gets the current console width.
size_t GetConsoleWidth();

// The base stream for all channels.
struct BaseStream
{
Expand Down
39 changes: 23 additions & 16 deletions src/AppInstallerCLICore/ExecutionProgress.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,23 @@ namespace AppInstaller::CLI::Execution
LOG_HR(E_UNEXPECTED);
}
}

void ProgressVisualizerBase::ClearLine()
{
if (UseVT())
{
m_out << TextModification::EraseLineEntirely << '\r';
}
else
{
m_out << '\r' << std::string(GetConsoleWidth(), ' ') << '\r';
}
}

void ProgressVisualizerBase::SetMessage(std::string_view message)
{
m_message = message;
}
}

void IndefiniteSpinner::ShowSpinner()
Expand Down Expand Up @@ -188,17 +205,19 @@ namespace AppInstaller::CLI::Execution
}

// Indent two spaces for the spinner, but three here so that we can overwrite it in the loop.
m_out << " ";
std::string_view indent = " ";

for (size_t i = 0; !m_canceled; ++i)
{
constexpr size_t repetitionCount = 20;
ApplyStyle(i % repetitionCount, repetitionCount, true);
m_out << '\b' << spinnerChars[i % ARRAYSIZE(spinnerChars)] << std::flush;
m_out << '\r' << indent << spinnerChars[i % ARRAYSIZE(spinnerChars)];
m_out.RestoreDefault();
m_out << ' ' << m_message << std::flush;
Sleep(250);
}

m_out << "\b \r";
ClearLine();

if (UseVT())
{
Expand All @@ -217,6 +236,7 @@ namespace AppInstaller::CLI::Execution
ClearLine();
}

// TODO: Progress bar does not currently use message
if (UseVT())
{
ShowProgressWithVT(current, maximum, type);
Expand Down Expand Up @@ -255,19 +275,6 @@ namespace AppInstaller::CLI::Execution
}
}

void ProgressBar::ClearLine()
{
if (UseVT())
{
m_out << TextModification::EraseLineEntirely << '\r';
}
else
{
// Best effort when no VT (arbitrary number of spaces that seems to work)
m_out << "\r \r";
}
}

void ProgressBar::ShowProgressNoVT(uint64_t current, uint64_t maximum, ProgressType type)
{
m_out << "\r ";
Expand Down
9 changes: 5 additions & 4 deletions src/AppInstallerCLICore/ExecutionProgress.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,20 @@ namespace AppInstaller::CLI::Execution

void SetStyle(AppInstaller::Settings::VisualStyle style) { m_style = style; }

void SetMessage(std::string_view message);

protected:
BaseStream& m_out;
Settings::VisualStyle m_style = AppInstaller::Settings::VisualStyle::Accent;
std::string m_message;

bool UseVT() const { return m_enableVT && m_style != AppInstaller::Settings::VisualStyle::NoVT; }

// Applies the selected visual style.
void ApplyStyle(size_t i, size_t max, bool foregroundOnly);

void ClearLine();

private:
bool m_enableVT = false;
};
Expand Down Expand Up @@ -70,14 +75,10 @@ namespace AppInstaller::CLI::Execution

void EndProgress(bool hideProgressWhenDone);

void SetStyle(AppInstaller::Settings::VisualStyle style) { m_style = style; }

private:
std::atomic<bool> m_isVisible = false;
uint64_t m_lastCurrent = 0;

void ClearLine();

void ShowProgressNoVT(uint64_t current, uint64_t maximum, ProgressType type);

void ShowProgressWithVT(uint64_t current, uint64_t maximum, ProgressType type);
Expand Down
14 changes: 14 additions & 0 deletions src/AppInstallerCLICore/ExecutionReporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,19 @@ namespace AppInstaller::CLI::Execution
}
}

void Reporter::SetProgressMessage(std::string_view message)
{
if (m_spinner)
{
m_spinner->SetMessage(message);
}

if (m_progressBar)
{
m_progressBar->SetMessage(message);
}
}

void Reporter::BeginProgress()
{
GetBasicOutputStream() << VirtualTerminal::Cursor::Visibility::DisableShow;
Expand All @@ -229,6 +242,7 @@ namespace AppInstaller::CLI::Execution
{
m_progressBar->EndProgress(hideProgressWhenDone);
}
SetProgressMessage({});
GetBasicOutputStream() << VirtualTerminal::Cursor::Visibility::EnableShow;
};

Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/ExecutionReporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ namespace AppInstaller::CLI::Execution
// IProgressSink
void BeginProgress() override;
void OnProgress(uint64_t current, uint64_t maximum, ProgressType type) override;
void SetProgressMessage(std::string_view message) override;
void EndProgress(bool hideProgressWhenDone) override;

// Runs the given callable of type: auto(IProgressCallback&)
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/Resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(InstallFlowStartingPackageInstall);
WINGET_DEFINE_RESOURCE_STRINGID(InstallLocationNotProvided);
WINGET_DEFINE_RESOURCE_STRINGID(InstallScopeDescription);
WINGET_DEFINE_RESOURCE_STRINGID(InstallWaitingOnAnother);
WINGET_DEFINE_RESOURCE_STRINGID(InteractiveArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(InvalidAliasError);
WINGET_DEFINE_RESOURCE_STRINGID(InvalidArgumentSpecifierError);
Expand Down
19 changes: 1 addition & 18 deletions src/AppInstallerCLICore/TableOutput.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,6 @@

namespace AppInstaller::CLI::Execution
{
namespace details
{
// Gets the column width of the console.
inline size_t GetConsoleWidth()
{
CONSOLE_SCREEN_BUFFER_INFO consoleInfo{};
if (GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &consoleInfo))
{
return static_cast<size_t>(consoleInfo.dwSize.X);
}
else
{
return 120;
}
}
}

// Enables output data in a table format.
// TODO: Improve for use with sparse data.
template <size_t FieldCount>
Expand Down Expand Up @@ -144,7 +127,7 @@ namespace AppInstaller::CLI::Execution
totalRequired += m_columns[i].MaxLength + (m_columns[i].SpaceAfter ? 1 : 0);
}

size_t consoleWidth = details::GetConsoleWidth();
size_t consoleWidth = GetConsoleWidth();

// If the total space would be too big, shrink them.
// We don't want to use the last column, lest we auto-wrap
Expand Down
Loading

0 comments on commit 741244e

Please sign in to comment.