Additional service hardening

build/staging/version/BundleInfo.wxi

@@ -1,4 +1,4 @@
 <Include>
   <?define SetupVersionName="Developer Preview 6 arm64" ?>
-  <?define SetupVersionNumber="1.0.24105.1327" ?>
+  <?define SetupVersionNumber="1.0.24107.1653" ?>
 </Include>

src/api/Abstraction/BleMidiAbstraction/Midi2.BluetoothMidiEndpointManager.cpp

@@ -27,6 +27,15 @@ CMidi2BluetoothMidiEndpointManager::Initialize(
     IUnknown* /*midiEndpointProtocolManager*/
 )
 {
+
+    // temporary because this may be crashing the service on start
+    return S_OK;
+
+
+
+
+
+
     try
     {
         TraceLoggingWrite(
@@ -36,14 +45,13 @@ CMidi2BluetoothMidiEndpointManager::Initialize(
             TraceLoggingPointer(this, "this")
         );
 
-        RETURN_HR_IF(E_INVALIDARG, nullptr == MidiDeviceManager);
+        RETURN_HR_IF_NULL(E_INVALIDARG, MidiDeviceManager);
 
         RETURN_IF_FAILED(MidiDeviceManager->QueryInterface(__uuidof(IMidiDeviceManagerInterface), (void**)&m_MidiDeviceManager));
 
         m_TransportAbstractionId = AbstractionLayerGUID;    // this is needed so MidiSrv can instantiate the correct transport
         m_ContainerId = m_TransportAbstractionId;           // we use the transport ID as the container ID for convenience
 
-
         RETURN_IF_FAILED(CreateParentDevice());
 
         RETURN_IF_FAILED(StartAdvertisementWatcher());
@@ -261,13 +269,23 @@ CMidi2BluetoothMidiEndpointManager::Cleanup()
         TraceLoggingPointer(this, "this")
     );
 
-
     if (m_bleAdWatcher != nullptr)
     {
         m_bleAdWatcher.Stop();
         m_bleAdWatcher = nullptr;
+
+        m_AdvertisementReceived.revoke();
+        m_AdvertisementWatcherStopped.revoke();
     }
 
+    TraceLoggingWrite(
+        MidiBluetoothMidiAbstractionTelemetryProvider::Provider(),
+        __FUNCTION__,
+        TraceLoggingLevel(WINEVENT_LEVEL_INFO),
+        TraceLoggingPointer(this, "this"),
+        TraceLoggingWideString(L"Cleanup complete", "message")
+    );
+
     return S_OK;
 }
 
@@ -361,7 +379,12 @@ _Use_decl_annotations_
 HRESULT
 CMidi2BluetoothMidiEndpointManager::OnBleDeviceNameChanged(bt::BluetoothLEDevice device, foundation::IInspectable /*args*/)
 {
-
+    TraceLoggingWrite(
+        MidiBluetoothMidiAbstractionTelemetryProvider::Provider(),
+        __FUNCTION__,
+        TraceLoggingLevel(WINEVENT_LEVEL_INFO),
+        TraceLoggingPointer(this, "this")
+    );
 
 
     return S_OK;
@@ -479,6 +502,7 @@ CMidi2BluetoothMidiEndpointManager::OnBleDeviceConnectionStatusChanged(bt::Bluet
             RETURN_IF_FAILED(m_MidiDeviceManager->DeactivateEndpoint(instanceId.c_str()));
 
             entry->Definition.SetDeactivated();
+            entry->MidiDeviceBiDi = nullptr;
         }
         else
         {
@@ -539,7 +563,7 @@ CMidi2BluetoothMidiEndpointManager::OnBleAdvertisementReceived(
         definition.BluetoothAddress = device.BluetoothAddress();
         definition.TransportSuppliedName = device.Name();           // need to ensure we also wire up the name changed event handler for this 
 
-        MidiBluetoothEndpointEntry* entry{ nullptr };
+        std::shared_ptr<MidiBluetoothEndpointEntry> entry{ nullptr };
 
         // todo: Should lock the entry list during this
 
@@ -559,7 +583,8 @@ CMidi2BluetoothMidiEndpointManager::OnBleAdvertisementReceived(
 
             // wire up connection status changed event and store the token for revocation
 //            entry->ConnectionStatusChangedToken = device.ConnectionStatusChanged(winrt::auto_revoke, connectionStatusChangedHandler);
-            entry->Device.ConnectionStatusChanged(connectionStatusChangedHandler);
+            //entry->ConnectionStatusChangedRevoker = entry->Device.ConnectionStatusChanged(winrt::auto_revoke, connectionStatusChangedHandler);
+            entry->ConnectionStatusChangedRevoker = entry->Device.ConnectionStatusChanged(connectionStatusChangedHandler);
         }
         else
         {

src/api/Abstraction/BleMidiAbstraction/MidiEndpointTable.cpp

@@ -25,14 +25,22 @@ MidiEndpointTable& MidiEndpointTable::Current()
 
 
 _Use_decl_annotations_
-MidiBluetoothEndpointEntry*
-MidiEndpointTable::GetEndpointEntryForBluetoothAddress(_In_ uint64_t const bluetoothAddress) const noexcept
+std::shared_ptr<MidiBluetoothEndpointEntry>
+MidiEndpointTable::GetEndpointEntryForBluetoothAddress(uint64_t const bluetoothAddress) const noexcept
 {
+    TraceLoggingWrite(
+        MidiBluetoothMidiAbstractionTelemetryProvider::Provider(),
+        __FUNCTION__,
+        TraceLoggingLevel(WINEVENT_LEVEL_INFO),
+        TraceLoggingPointer(this, "this"),
+        TraceLoggingUInt64(bluetoothAddress, "address")
+    );
+
     try
     {
         if (auto it = m_endpoints.find(bluetoothAddress); it != m_endpoints.end())
         {
-            return it->second.get();
+            return it->second;
         }
         else
         {
@@ -46,37 +54,22 @@ MidiEndpointTable::GetEndpointEntryForBluetoothAddress(_In_ uint64_t const bluet
 }
 
 
-
-//_Use_decl_annotations_
-//wil::com_ptr_nothrow<IMidiBiDi>
-//MidiEndpointTable::GetEndpointInterfaceForId(
-//    std::wstring const EndpointDeviceId
-//) const noexcept
-//{
-//    try
-//    {
-//        auto result = m_endpoints.find(EndpointDeviceId);
-//
-//        if (result != m_endpoints.end())
-//            return result->second.MidiDeviceBiDi;
-//        else
-//            return nullptr;
-//    }
-//    catch (...)
-//    {
-//        return nullptr;
-//    }
-//}
-
-
 _Use_decl_annotations_
-MidiBluetoothEndpointEntry*
+std::shared_ptr<MidiBluetoothEndpointEntry>
 MidiEndpointTable::CreateAndAddNewEndpointEntry(
     MidiBluetoothDeviceDefinition definition,
     bt::BluetoothLEDevice device,
     gatt::GattDeviceService service
 ) noexcept
 {
+    TraceLoggingWrite(
+        MidiBluetoothMidiAbstractionTelemetryProvider::Provider(),
+        __FUNCTION__,
+        TraceLoggingLevel(WINEVENT_LEVEL_INFO),
+        TraceLoggingPointer(this, "this"),
+        TraceLoggingUInt64(definition.BluetoothAddress, "address")
+    );
+
     try
     {
         MidiBluetoothEndpointEntry entry;
@@ -88,7 +81,7 @@ MidiEndpointTable::CreateAndAddNewEndpointEntry(
 
         m_endpoints[definition.BluetoothAddress] = std::make_shared<MidiBluetoothEndpointEntry>(entry);
 
-        return m_endpoints[definition.BluetoothAddress].get();
+        return m_endpoints[definition.BluetoothAddress];
     }
     catch (...)
     {
@@ -105,6 +98,14 @@ MidiEndpointTable::RemoveEndpointEntry(
     uint64_t bluetoothAddress
 ) noexcept
 {
+    TraceLoggingWrite(
+        MidiBluetoothMidiAbstractionTelemetryProvider::Provider(),
+        __FUNCTION__,
+        TraceLoggingLevel(WINEVENT_LEVEL_INFO),
+        TraceLoggingPointer(this, "this"),
+        TraceLoggingUInt64(bluetoothAddress, "address")
+    );
+
     try
     {
         auto result = m_endpoints.find(bluetoothAddress);

src/api/Abstraction/BleMidiAbstraction/MidiEndpointTable.h

@@ -22,15 +22,27 @@ struct MidiBluetoothEndpointEntry
 
     wil::com_ptr_nothrow<IMidiBiDi> MidiDeviceBiDi{ nullptr };
 
- //   winrt::impl::consume_Windows_Devices_Bluetooth_IBluetoothLEDevice<bt::IBluetoothLEDevice>::ConnectionStatusChanged_revoker ConnectionStatusChangedToken{};
+    //winrt::event_token ConnectionStatusChangedRevoker;
+
+    //bt::BluetoothLEDevice::ConnectionParametersChanged_revoker ConnectionStatusChangedRevoker{ nullptr };
+    //winrt::impl::consume_Windows_Devices_Bluetooth_IBluetoothLEDevice<bt::IBluetoothLEDevice>::ConnectionStatusChanged_revoker ConnectionStatusChangedRevoker{ nullptr };
+//    winrt::impl::consume_Windows_Devices_Bluetooth_IBluetoothLEDevice<bt::IBluetoothLEDevice>::ConnectionStatusChanged_revoker ConnectionStatusChangedRevoker;
+
+    winrt::event_token ConnectionStatusChangedRevoker;
 
     MidiBluetoothEndpointEntry() = default;
     ~MidiBluetoothEndpointEntry()
     {
         if (MidiDeviceBiDi)
         {
+            MidiDeviceBiDi->Cleanup();
             MidiDeviceBiDi.reset();
         }
+
+        if (Device != nullptr)
+        {
+            Device.ConnectionStatusChanged(ConnectionStatusChangedRevoker);
+        }
     }
 };
 
@@ -46,9 +58,9 @@ class MidiEndpointTable
 
 //    wil::com_ptr_nothrow<IMidiBiDi> GetEndpointInterfaceForId(_In_ std::wstring const EndpointDeviceId) const noexcept;
 
-    MidiBluetoothEndpointEntry* GetEndpointEntryForBluetoothAddress(_In_ uint64_t const bluetoothAddress) const noexcept;
+    std::shared_ptr<MidiBluetoothEndpointEntry> GetEndpointEntryForBluetoothAddress(_In_ uint64_t const bluetoothAddress) const noexcept;
 
-    MidiBluetoothEndpointEntry* CreateAndAddNewEndpointEntry(_In_ MidiBluetoothDeviceDefinition definition, _In_  bt::BluetoothLEDevice device, _In_ gatt::GattDeviceService service) noexcept;
+    std::shared_ptr<MidiBluetoothEndpointEntry> CreateAndAddNewEndpointEntry(_In_ MidiBluetoothDeviceDefinition definition, _In_  bt::BluetoothLEDevice device, _In_ gatt::GattDeviceService service) noexcept;
 
     void RemoveEndpointEntry(_In_ uint64_t bluetoothAddress) noexcept;
 

src/api/Abstraction/KSAbstraction/Midi2.KSMidiEndpointManager.cpp

@@ -601,14 +601,22 @@ HRESULT CMidi2KSMidiEndpointManager::OnDeviceAdded(DeviceWatcher watcher, Device
                 BYTE preferredProtocol{ MIDI_PROP_CONFIGURED_PROTOCOL_MIDI2 };
                 WORD negotiationTimeoutMS{ 5000 };  // this should be much shorter
 
+                PENDPOINTPROTOCOLNEGOTIATIONRESULTS negotiationResults;
+
                 LOG_IF_FAILED(m_MidiProtocolManager->NegotiateAndRequestMetadata(
                     newDeviceInterfaceId,
                     preferToSendJRToEndpoint,
                     preferToReceiveJRFromEndpoint,
                     preferredProtocol,
                     negotiationTimeoutMS,
-                    true
+                    &negotiationResults
                 ));
+
+
+                // TODO: The negotiationResults structure contains the function blocks which 
+                // should be used to create MIDI 1.0 legacy ports for this MIDI 2.0 device.
+
+
             }
 
         }

src/api/Abstraction/VirtualMidiAbstraction/Midi2.VirtualMidiEndpointManager.cpp

@@ -142,7 +142,7 @@ CMidi2VirtualMidiEndpointManager::CreateParentDevice()
 {
     TraceLoggingWrite(
         MidiVirtualMidiAbstractionTelemetryProvider::Provider(),
-        __func__,
+        __FUNCTION__,
         TraceLoggingLevel(WINEVENT_LEVEL_INFO),
         TraceLoggingPointer(this, "this")
     );
@@ -202,13 +202,15 @@ CMidi2VirtualMidiEndpointManager::NegotiateAndRequestMetadata(std::wstring endpo
     BYTE preferredProtocol{ MIDI_PROP_CONFIGURED_PROTOCOL_MIDI2 };
     WORD negotiationTimeoutMS{ 2000 };
 
+    PENDPOINTPROTOCOLNEGOTIATIONRESULTS negotiationResults;
+
     RETURN_IF_FAILED(m_MidiProtocolManager->NegotiateAndRequestMetadata(
         endpointId.c_str(),
         preferToSendJRToEndpoint,
         preferToReceiveJRFromEndpoint,
         preferredProtocol,
         negotiationTimeoutMS, 
-        true
+        &negotiationResults
     ));
 
     return S_OK;

src/api/Client/Midi2Client/trace_logging.cpp

@@ -77,7 +77,7 @@ namespace Windows::Devices::Midi2::Internal
         const wchar_t* message, 
         winrt::hresult_error const& ex) noexcept
     {
-        OutputDebugString(L"" __FUNCTION__ L"API HRESULT Error. Use tracing provider for details.");
+        //OutputDebugString(L"" __FUNCTION__ L"API HRESULT Error. Use tracing provider for details.");
 
         if (!g_traceLoggingRegistered) RegisterTraceLogging();
 
@@ -98,13 +98,14 @@ namespace Windows::Devices::Midi2::Internal
         const char* location, 
         const wchar_t* message) noexcept
     {
-        OutputDebugString(L"" __FUNCTION__ L"API General Error. Use tracing provider for details.");
+        //OutputDebugString(L"" __FUNCTION__ L"API General Error. Use tracing provider for details.");
 
         if (!g_traceLoggingRegistered) RegisterTraceLogging();
 
         TraceLoggingWrite(
             g_hLoggingProvider,
             "MIDI.GeneralError",
+            TraceLoggingOpcode(ERROR),
             TraceLoggingLevel(WINEVENT_LEVEL_ERROR),
             TraceLoggingKeyword(TRACE_KEYWORD_API_GENERAL),
             TraceLoggingString(location, "Location"),