Skip to content

Commit

Permalink
Add try/catch's throughout Monarch.cpp
Browse files Browse the repository at this point in the history
  • Loading branch information
@zadjii-msft
zadjii-msft committed Jan 27, 2021
1 parent b2db317 commit d2a3438
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 54 deletions.
4 changes: 2 additions & 2 deletions src/cascadia/Remoting/CommandlineArgs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
// It must be defined after CommandlineArgs.g.cpp, otherwise the compiler
// will give you just the most impossible template errors to try and
// decipher.
void CommandlineArgs::Args(winrt::array_view<const winrt::hstring> const& value)
void CommandlineArgs::Commandline(winrt::array_view<const winrt::hstring> const& value)
{
_args = { value.begin(), value.end() };
}

winrt::com_array<winrt::hstring> CommandlineArgs::Args()
winrt::com_array<winrt::hstring> CommandlineArgs::Commandline()
{
return winrt::com_array<winrt::hstring>{ _args.begin(), _args.end() };
}
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/Remoting/CommandlineArgs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation

winrt::hstring CurrentDirectory() { return _cwd; };

void Args(winrt::array_view<const winrt::hstring> const& value);
winrt::com_array<winrt::hstring> Args();
void Commandline(winrt::array_view<const winrt::hstring> const& value);
winrt::com_array<winrt::hstring> Commandline();

private:
winrt::com_array<winrt::hstring> _args;
Expand Down
127 changes: 81 additions & 46 deletions src/cascadia/Remoting/Monarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,35 +46,48 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
// - the ID assigned to the peasant.
uint64_t Monarch::AddPeasant(Remoting::IPeasant peasant)
{
// TODO:projects/5 This is terrible. There's gotta be a better way
// of finding the first opening in a non-consecutive map of int->object
const auto providedID = peasant.GetID();

if (providedID == 0)
{
// Peasant doesn't currently have an ID. Assign it a new one.
peasant.AssignID(_nextPeasantID++);
}
else
try
{
// Peasant already had an ID (from an older monarch). Leave that one
// be. Make sure that the next peasant's ID is higher than it.
_nextPeasantID = providedID >= _nextPeasantID ? providedID + 1 : _nextPeasantID;
}
// TODO:projects/5 This is terrible. There's gotta be a better way
// of finding the first opening in a non-consecutive map of int->object
const auto providedID = peasant.GetID();

auto newPeasantsId = peasant.GetID();
_peasants[newPeasantsId] = peasant;
if (providedID == 0)
{
// Peasant doesn't currently have an ID. Assign it a new one.
peasant.AssignID(_nextPeasantID++);
}
else
{
// Peasant already had an ID (from an older monarch). Leave that one
// be. Make sure that the next peasant's ID is higher than it.
_nextPeasantID = providedID >= _nextPeasantID ? providedID + 1 : _nextPeasantID;
}

// Add an event listener to the peasant's WindowActivated event.
peasant.WindowActivated({ this, &Monarch::_peasantWindowActivated });
auto newPeasantsId = peasant.GetID();
// Add an event listener to the peasant's WindowActivated event.
peasant.WindowActivated({ this, &Monarch::_peasantWindowActivated });

TraceLoggingWrite(g_hRemotingProvider,
"Monarch_AddPeasant",
TraceLoggingUInt64(providedID, "providedID", "the provided ID for the peasant"),
TraceLoggingUInt64(newPeasantsId, "peasantID", "the ID of the new peasant"),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
_peasants[newPeasantsId] = peasant;

return newPeasantsId;
TraceLoggingWrite(g_hRemotingProvider,
"Monarch_AddPeasant",
TraceLoggingUInt64(providedID, "providedID", "the provided ID for the peasant"),
TraceLoggingUInt64(newPeasantsId, "peasantID", "the ID of the new peasant"),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
return newPeasantsId;
}
catch (...)
{
TraceLoggingWrite(g_hRemotingProvider,
"Monarch_AddPeasant_Failed",
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));

// We can only get into this try/catch if the peasant died on us. So
// the return value doesn't _really_ matter. They're not about to
// get it.
return -1;
}
}

// Method Description:
Expand Down Expand Up @@ -104,6 +117,8 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
{
auto peasantSearch = _peasants.find(peasantID);
auto maybeThePeasant = peasantSearch == _peasants.end() ? nullptr : peasantSearch->second;
// Ask the peasant for their PID. This will validate that they're
// actually still alive.
if (maybeThePeasant)
{
maybeThePeasant.GetPID();
Expand Down Expand Up @@ -148,23 +163,31 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation

uint64_t Monarch::_getMostRecentPeasantID()
{
if (_mostRecentPeasant == 0)
if (_mostRecentPeasant != 0)
{
// We haven't yet been told the MRU peasant. Just use the first one.
// This is just gonna be a random one, but really shouldn't happen
// in practice. The WindowManager should set the MRU peasant
// immediately as soon as it creates the monarch/peasant for the
// first window.
if (_peasants.size() > 0)
return _mostRecentPeasant;
}

// We haven't yet been told the MRU peasant. Just use the first one.
// This is just gonna be a random one, but really shouldn't happen
// in practice. The WindowManager should set the MRU peasant
// immediately as soon as it creates the monarch/peasant for the
// first window.
if (_peasants.size() > 0)
{
try
{
return _peasants.begin()->second.GetID();
}
return 0;
}
else
{
return _mostRecentPeasant;
catch (...)
{
// This shouldn't really happen. If we're the monarch, then the
// first peasant should also _be us_. So we should be able to
// get our own ID.
return 0;
}
}
return 0;
}

// Method Description:
Expand All @@ -187,6 +210,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
auto findWindowArgs = winrt::make_self<Remoting::implementation::FindTargetWindowArgs>();
findWindowArgs->Args(args);

// This is handled by some handler in-proc
_FindTargetWindowRequestedHandlers(*this, *findWindowArgs);

// After the event was handled, ResultTargetWindow() will be filled with
Expand All @@ -205,20 +229,31 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation

if (auto targetPeasant{ _getPeasant(windowID) })
{
// This will raise the peasant's ExecuteCommandlineRequested
// event, which will then ask the AppHost to handle the
// commandline, which will then pass it to AppLogic for
// handling.
targetPeasant.ExecuteCommandline(args);
auto result = winrt::make_self<Remoting::implementation::ProposeCommandlineResult>();

result->ShouldCreateWindow(false);
try
{
// This will raise the peasant's ExecuteCommandlineRequested
// event, which will then ask the AppHost to handle the
// commandline, which will then pass it to AppLogic for
// handling.
targetPeasant.ExecuteCommandline(args);
}
catch (...)
{
// If we fail to propose the commandline to the peasant (it
// died?) then just tell this process to become a new window
// instead.
result->ShouldCreateWindow(false);
}

TraceLoggingWrite(g_hRemotingProvider,
"Monarch_ProposeCommandline_Existing",
TraceLoggingUInt64(windowID, "peasantID", "the ID of the matching peasant"),
TraceLoggingUInt64(windowID, "peasantID", "the ID of the peasant the commandline waws intended for"),
TraceLoggingBoolean(true, "foundMatch", "true if we found a peasant with that ID"),
TraceLoggingBoolean(!result->ShouldCreateWindow(), "succeeded", "true if we successfully dispatched the commandline to the peasant"),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));

auto result = winrt::make_self<Remoting::implementation::ProposeCommandlineResult>();
result->ShouldCreateWindow(false);
return *result;
}
else if (windowID > 0)
Expand All @@ -229,7 +264,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation

TraceLoggingWrite(g_hRemotingProvider,
"Monarch_ProposeCommandline_Existing",
TraceLoggingUInt64(windowID, "peasantID", "the ID of the matching peasant"),
TraceLoggingUInt64(windowID, "peasantID", "the ID of the peasant the commandline waws intended for"),
TraceLoggingBoolean(false, "foundMatch", "true if we found a peasant with that ID"),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/Remoting/Peasant.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Microsoft.Terminal.Remoting
CommandlineArgs();
CommandlineArgs(String[] args, String cwd);

String[] Args { get; set; };
String[] Commandline { get; set; };
String CurrentDirectory();
};

Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/WindowsTerminal/AppHost.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ void AppHost::_HandleCommandlineArgs()
{
if (auto args{ peasant.InitialArgs() })
{
const auto result = _logic.SetStartupCommandline(args.Args());
const auto result = _logic.SetStartupCommandline(args.Commandline());
const auto message = _logic.ParseCommandlineMessage();
if (!message.empty())
{
Expand Down Expand Up @@ -530,7 +530,7 @@ bool AppHost::HasWindow()
void AppHost::_DispatchCommandline(winrt::Windows::Foundation::IInspectable /*sender*/,
Remoting::CommandlineArgs args)
{
_logic.ExecuteCommandline(args.Args(), args.CurrentDirectory());
_logic.ExecuteCommandline(args.Commandline(), args.CurrentDirectory());
}

// Method Description:
Expand All @@ -546,7 +546,7 @@ void AppHost::_DispatchCommandline(winrt::Windows::Foundation::IInspectable /*se
void AppHost::_FindTargetWindow(const winrt::Windows::Foundation::IInspectable& /*sender*/,
const Remoting::FindTargetWindowArgs& args)
{
const auto targetWindow = _logic.FindTargetWindow(args.Args().Args());
const auto targetWindow = _logic.FindTargetWindow(args.Args().Commandline());
args.ResultTargetWindow(targetWindow);
}

Expand Down

0 comments on commit d2a3438

Please sign in to comment.