Skip to content

Commit

Permalink
Skip DECPS/MIDI output on Ctrl+C/Break (#14214)
Browse files Browse the repository at this point in the history
Silent MIDI notes can be used to seemingly deny a user's input for long
durations (multiple minutes). This commit improves the situation by ignoring
all DECPS sequences for a second when Ctrl+C/Ctrl+Break is pressed.

Additionally it fixes a regression introduced in 666c446:
When we close a tab we need to unblock/shutdown `MidiAudio` early,
so that `ConptyConnection::Close()` can run down as fast as possible.

* In pwsh in Windows Terminal 1.16 run ``while ($True) { echo "`e[3;8;3,~" }``
  * Ctrl+C doesn't do anything ❎
  * Closing the tab doesn't do anything ❎
* With these modifications in Windows Terminal:
  * Ctrl+C stops the output ✅
  * Closing the tab completes instantly ✅
* With these modifications in OpenConsole:
  * Ctrl+C stops the output ✅
  * Closing the window completes instantly ✅

(cherry picked from commit b4fce27)
Service-Card-Id: 86518584
Service-Version: 1.15
  • Loading branch information
lhecker authored and DHowett committed Dec 1, 2022
1 parent 4f1a72a commit 716cb56
Show file tree
Hide file tree
Showing 12 changed files with 111 additions and 162 deletions.
75 changes: 24 additions & 51 deletions src/audio/midi/MidiAudio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
#include "MidiAudio.hpp"
#include "../terminal/parser/stateMachine.hpp"

#include <dsound.h>

#pragma comment(lib, "dxguid.lib")

using Microsoft::WRL::ComPtr;
using namespace std::chrono_literals;

Expand All @@ -17,71 +13,48 @@ using namespace std::chrono_literals;
constexpr auto WAVE_SIZE = 16u;
constexpr auto WAVE_DATA = std::array<byte, WAVE_SIZE>{ 128, 159, 191, 223, 255, 223, 191, 159, 128, 96, 64, 32, 0, 32, 64, 96 };

MidiAudio::MidiAudio(HWND windowHandle)
void MidiAudio::_initialize(HWND windowHandle) noexcept
{
_hwnd = windowHandle;
_directSoundModule.reset(LoadLibraryExW(L"dsound.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32));
if (_directSoundModule)
{
auto createFunction = GetProcAddressByFunctionDeclaration(_directSoundModule.get(), DirectSoundCreate8);
if (SUCCEEDED(createFunction(nullptr, &_directSound, nullptr)))
if (const auto createFunction = GetProcAddressByFunctionDeclaration(_directSoundModule.get(), DirectSoundCreate8))
{
if (SUCCEEDED(_directSound->SetCooperativeLevel(windowHandle, DSSCL_NORMAL)))
if (SUCCEEDED(createFunction(nullptr, &_directSound, nullptr)))
{
_createBuffers();
if (SUCCEEDED(_directSound->SetCooperativeLevel(windowHandle, DSSCL_NORMAL)))
{
_createBuffers();
}
}
}
}
}

MidiAudio::~MidiAudio() noexcept
{
try
{
#pragma warning(suppress : 26447)
// We acquire the lock here so the class isn't destroyed while in use.
// If this throws, we'll catch it, so the C26447 warning is bogus.
const auto lock = std::unique_lock{ _inUseMutex };
}
catch (...)
{
// If the lock fails, we'll just have to live with the consequences.
}
}

void MidiAudio::Initialize()
void MidiAudio::BeginSkip() noexcept
{
_shutdownFuture = _shutdownPromise.get_future();
_skip.SetEvent();
}

void MidiAudio::Shutdown()
void MidiAudio::EndSkip() noexcept
{
// Once the shutdown promise is set, any note that is playing will stop
// immediately, and the Unlock call will exit the thread ASAP.
_shutdownPromise.set_value();
_skip.ResetEvent();
}

void MidiAudio::Lock()
void MidiAudio::PlayNote(HWND windowHandle, const int noteNumber, const int velocity, const std::chrono::milliseconds duration) noexcept
try
{
_inUseMutex.lock();
}
if (_skip.is_signaled())
{
return;
}

void MidiAudio::Unlock()
{
// We need to check the shutdown status before releasing the mutex,
// because after that the class could be destroyed.
const auto shutdownStatus = _shutdownFuture.wait_for(0s);
_inUseMutex.unlock();
// If the wait didn't timeout, that means the shutdown promise was set,
// so we need to exit the thread ASAP by throwing an exception.
if (shutdownStatus != std::future_status::timeout)
if (_hwnd != windowHandle)
{
throw Microsoft::Console::VirtualTerminal::StateMachine::ShutdownException{};
_initialize(windowHandle);
}
}

void MidiAudio::PlayNote(const int noteNumber, const int velocity, const std::chrono::microseconds duration) noexcept
try
{
const auto& buffer = _buffers.at(_activeBufferIndex);
if (velocity && buffer)
{
Expand All @@ -104,10 +77,10 @@ try
buffer->SetCurrentPosition((_lastBufferPosition + 12) % WAVE_SIZE);
}

// By waiting on the shutdown future with the duration of the note, we'll
// either be paused for the appropriate amount of time, or we'll break out
// of the wait early if we've been shutdown.
_shutdownFuture.wait_for(duration);
// By waiting on the skip event with a maximum duration of the note, we'll
// either be paused for the appropriate amount of time, or we'll break out early
// because BeginSkip() was called. This happens for Ctrl+C or during shutdown.
_skip.wait(::base::saturated_cast<DWORD>(duration.count()));

if (velocity && buffer)
{
Expand Down
27 changes: 9 additions & 18 deletions src/audio/midi/MidiAudio.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,36 +12,27 @@ Module Name:
#pragma once

#include <array>
#include <future>
#include <mutex>

struct IDirectSound8;
struct IDirectSoundBuffer;

class MidiAudio
{
public:
MidiAudio(HWND windowHandle);
MidiAudio(const MidiAudio&) = delete;
MidiAudio(MidiAudio&&) = delete;
MidiAudio& operator=(const MidiAudio&) = delete;
MidiAudio& operator=(MidiAudio&&) = delete;
~MidiAudio() noexcept;
void Initialize();
void Shutdown();
void Lock();
void Unlock();
void PlayNote(const int noteNumber, const int velocity, const std::chrono::microseconds duration) noexcept;
void BeginSkip() noexcept;
void EndSkip() noexcept;
void PlayNote(HWND windowHandle, const int noteNumber, const int velocity, const std::chrono::milliseconds duration) noexcept;

private:
void _initialize(HWND windowHandle) noexcept;
void _createBuffers() noexcept;

wil::slim_event_manual_reset _skip;

HWND _hwnd = nullptr;
wil::unique_hmodule _directSoundModule;
Microsoft::WRL::ComPtr<IDirectSound8> _directSound;
std::array<Microsoft::WRL::ComPtr<IDirectSoundBuffer>, 2> _buffers;
wil::com_ptr<IDirectSound8> _directSound;
std::array<wil::com_ptr<IDirectSoundBuffer>, 2> _buffers;
size_t _activeBufferIndex = 0;
DWORD _lastBufferPosition = 0;
std::promise<void> _shutdownPromise;
std::future<void> _shutdownFuture;
std::mutex _inUseMutex;
};
4 changes: 3 additions & 1 deletion src/audio/midi/precomp.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ Module Name:
#endif

// Windows Header Files:
#include <windows.h>
#include <Windows.h>

#include <mmeapi.h>
#include <dsound.h>

// clang-format on
80 changes: 33 additions & 47 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
#include "pch.h"
#include "ControlCore.h"

// MidiAudio
#include <mmeapi.h>
#include <dsound.h>

#include <DefaultSettings.h>
#include <unicode.hpp>
#include <Utf16Parser.hpp>
Expand Down Expand Up @@ -228,8 +232,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
_renderer->TriggerTeardown();
}

_shutdownMidiAudio();
}

bool ControlCore::Initialize(const double actualWidth,
Expand Down Expand Up @@ -388,9 +390,33 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const WORD scanCode,
const ::Microsoft::Terminal::Core::ControlKeyStates modifiers)
{
if (ch == L'\x3') // Ctrl+C or Ctrl+Break
{
_handleControlC();
}

return _terminal->SendCharEvent(ch, scanCode, modifiers);
}

void ControlCore::_handleControlC()
{
if (!_midiAudioSkipTimer)
{
_midiAudioSkipTimer = _dispatcher.CreateTimer();
_midiAudioSkipTimer.Interval(std::chrono::seconds(1));
_midiAudioSkipTimer.IsRepeating(false);
_midiAudioSkipTimer.Tick([weakSelf = get_weak()](auto&&, auto&&) {
if (const auto self = weakSelf.get())
{
self->_midiAudio.EndSkip();
}
});
}

_midiAudio.BeginSkip();
_midiAudioSkipTimer.Start();
}

bool ControlCore::_shouldTryUpdateSelection(const WORD vkey)
{
// GH#6423 - don't update selection if the key that was pressed was a
Expand Down Expand Up @@ -1345,57 +1371,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - duration - How long the note should be sustained (in microseconds).
void ControlCore::_terminalPlayMidiNote(const int noteNumber, const int velocity, const std::chrono::microseconds duration)
{
// We create the audio instance on demand, and lock it for the duration
// of the note output so it can't be destroyed while in use.
auto& midiAudio = _getMidiAudio();
midiAudio.Lock();

// We then unlock the terminal, so the UI doesn't hang while we're busy.
// Unlock the terminal, so the UI doesn't hang while we're busy.
auto& terminalLock = _terminal->GetReadWriteLock();
terminalLock.unlock();

// This call will block for the duration, unless shutdown early.
midiAudio.PlayNote(noteNumber, velocity, duration);
_midiAudio.PlayNote(reinterpret_cast<HWND>(_owningHwnd), noteNumber, velocity, std::chrono::duration_cast<std::chrono::milliseconds>(duration));

// Once complete, we reacquire the terminal lock and unlock the audio.
// If the terminal has shutdown in the meantime, the Unlock call
// will throw an exception, forcing the thread to exit ASAP.
terminalLock.lock();
midiAudio.Unlock();
}

// Method Description:
// - Returns the MIDI audio instance, created on demand.
// Arguments:
// - <none>
// Return Value:
// - a reference to the MidiAudio instance.
MidiAudio& ControlCore::_getMidiAudio()
{
if (!_midiAudio)
{
const auto windowHandle = reinterpret_cast<HWND>(_owningHwnd);
_midiAudio = std::make_unique<MidiAudio>(windowHandle);
_midiAudio->Initialize();
}
return *_midiAudio;
}

// Method Description:
// - Shuts down the MIDI audio system if previously instantiated.
// Arguments:
// - <none>
// Return Value:
// - <none>
void ControlCore::_shutdownMidiAudio()
{
if (_midiAudio)
{
// We lock the terminal here to make sure the shutdown promise is
// set before the audio is unlocked in the thread that is playing.
auto lock = _terminal->LockForWriting();
_midiAudio->Shutdown();
}
}

bool ControlCore::HasSelection() const
Expand Down Expand Up @@ -1480,6 +1463,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
_closing = true;

// Ensure Close() doesn't hang, waiting for MidiAudio to finish playing an hour long song.
_midiAudio.BeginSkip();

// Stop accepting new output and state changes before we disconnect everything.
_connection.TerminalOutput(_connectionOutputEventToken);
_connectionStateChangedRevoker.revoke();
Expand Down
7 changes: 3 additions & 4 deletions src/cascadia/TerminalControl/ControlCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void _updateSelectionUI();
bool _shouldTryUpdateSelection(const WORD vkey);

void _handleControlC();
void _sendInputToConnection(std::wstring_view wstr);

#pragma region TerminalCoreCallbacks
Expand All @@ -292,10 +293,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const std::chrono::microseconds duration);
#pragma endregion

std::unique_ptr<MidiAudio> _midiAudio;

MidiAudio& _getMidiAudio();
void _shutdownMidiAudio();
MidiAudio _midiAudio;
winrt::Windows::System::DispatcherQueueTimer _midiAudioSkipTimer{ nullptr };

#pragma region RendererCallbacks
void _rendererWarning(const HRESULT hr);
Expand Down
32 changes: 6 additions & 26 deletions src/host/consoleInformation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
#include "precomp.h"
#include <intsafe.h>

// MidiAudio
#include <mmeapi.h>
#include <dsound.h>

#include "misc.h"
#include "output.h"
#include "srvinit.h"
Expand Down Expand Up @@ -373,38 +377,14 @@ Microsoft::Console::CursorBlinker& CONSOLE_INFORMATION::GetCursorBlinker() noexc
}

// Method Description:
// - Returns the MIDI audio instance, created on demand.
// - Returns the MIDI audio instance.
// Arguments:
// - <none>
// Return Value:
// - a reference to the MidiAudio instance.
MidiAudio& CONSOLE_INFORMATION::GetMidiAudio()
{
if (!_midiAudio)
{
const auto windowHandle = ServiceLocator::LocateConsoleWindow()->GetWindowHandle();
_midiAudio = std::make_unique<MidiAudio>(windowHandle);
_midiAudio->Initialize();
}
return *_midiAudio;
}

// Method Description:
// - Shuts down the MIDI audio system if previously instantiated.
// Arguments:
// - <none>
// Return Value:
// - <none>
void CONSOLE_INFORMATION::ShutdownMidiAudio()
{
if (_midiAudio)
{
// We lock the console here to make sure the shutdown promise is
// set before the audio is unlocked in the thread that is playing.
LockConsole();
_midiAudio->Shutdown();
UnlockConsole();
}
return _midiAudio;
}

// Method Description:
Expand Down
4 changes: 4 additions & 0 deletions src/host/globals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@

#include "globals.h"

// MidiAudio
#include <mmeapi.h>
#include <dsound.h>

#pragma hdrstop

Globals::Globals()
Expand Down

0 comments on commit 716cb56

Please sign in to comment.