Repository: microsoft/terminal
File: src/cascadia/Remoting/WindowManager.cpp (C++)

Code before:
                _monarch.AddPeasant(_peasant);
                break;
            }
            catch (...)
            {
                try
                {
                    // Wrap this in it's own try/catch, because this can throw.
                    _createMonarchAndCallbacks();
                }
                catch (...)
                {
                }
            }
        }

        TraceLoggingWrite(g_hRemotingProvider,
                          "WindowManager_CreateOurPeasant",
                          TraceLoggingUInt64(_peasant.GetID(), "peasantID", "The ID of our new peasant"),
                          TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
                          TraceLoggingKeyword(TIL_KEYWORD_TRACE));

        // If the peasant asks us to quit we should not try to act in future elections.
        _peasant.QuitRequested([this](auto&&, auto&&) {
            _monarchWaitInterrupt.SetEvent();
        });

        return _peasant;
    }

    // Method Description:
    // - Attempt to connect to the monarch process. This might be us!
    // - For the new monarch, add us to their list of peasants.
    // Arguments:
    // - <none>
    // Return Value:
    // - true iff we're the new monarch process.
    // NOTE: This can throw!
    bool WindowManager::_performElection()
    {
        _createMonarchAndCallbacks();

        // Tell the new monarch who we are. We might be that monarch!
        _monarch.AddPeasant(_peasant);

        // This method is only called when a _new_ monarch is elected. So
        // don't do anything here that needs to be done for all monarch
        // windows. This should only be for work that's done when a window
        // _becomes_ a monarch, after the death of the previous monarch.
        return _isKing;
    }

Review comment (suggestion):
This callback isn't removed in the destructor.
But simultaneously `Peasant` is a reference counted class and might outlive `this`. We either need to remove the callback in our destructor, or use an explicit weak pointer instead of `this`.

Code after:
                _monarch.AddPeasant(_peasant);
                break;
            }
            catch (...)
            {
                try
                {
                    // Wrap this in it's own try/catch, because this can throw.
                    _createMonarchAndCallbacks();
                }
                catch (...)
                {
                }
            }
        }

        TraceLoggingWrite(g_hRemotingProvider,
                          "WindowManager_CreateOurPeasant",
                          TraceLoggingUInt64(_peasant.GetID(), "peasantID", "The ID of our new peasant"),
                          TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
                          TraceLoggingKeyword(TIL_KEYWORD_TRACE));

        // If the peasant asks us to quit we should not try to act in future elections.
        _peasant.QuitRequested([weakThis{ get_weak() }](auto&&, auto&&) {
            if (auto wm = weakThis.get())
            {
                wm->_monarchWaitInterrupt.SetEvent();
            }
        });

        return _peasant;
    }

    // Method Description:
    // - Attempt to connect to the monarch process. This might be us!
    // - For the new monarch, add us to their list of peasants.
    // Arguments:
    // - <none>
    // Return Value:
    // - true iff we're the new monarch process.
    // NOTE: This can throw!
    bool WindowManager::_performElection()
    {
        _createMonarchAndCallbacks();

        // Tell the new monarch who we are. We might be that monarch!
        _monarch.AddPeasant(_peasant);

        // This method is only called when a _new_ monarch is elected. So
        // don't do anything here that needs to be done for all monarch
        // windows. This should only be for work that's done when a window