Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Prototype] Implement createchannelasync for requesting channels #508

Closed
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions dev/PushNotifications/ChannelResult.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,26 @@
#include "pch.h"
#include "ChannelResult.h"
#include "ChannelResult.g.cpp"
#include <winrt\windows.networking.pushnotifications.h>

namespace winrt::Microsoft::ProjectReunion::implementation
{
ChannelResult::ChannelResult(Windows::Networking::PushNotifications::PushNotificationChannel const& channel, winrt::hresult const& extendedError, Microsoft::ProjectReunion::ChannelStatus const& status)
ChannelResult::ChannelResult(winrt::Windows::Networking::PushNotifications::PushNotificationChannel const channel, winrt::hresult const extendedError, Microsoft::ProjectReunion::ChannelStatus const status)
{
throw hresult_not_implemented();
m_channel = channel;
m_extendedError = extendedError;
m_channelStatus = status;
}
Windows::Networking::PushNotifications::PushNotificationChannel ChannelResult::Channel()
{
throw hresult_not_implemented();
return m_channel;
}
winrt::hresult ChannelResult::ExtendedError()
{
throw hresult_not_implemented();
return m_extendedError;
}
Microsoft::ProjectReunion::ChannelStatus ChannelResult::Status()
{
throw hresult_not_implemented();
return m_channelStatus;
}
}
7 changes: 6 additions & 1 deletion dev/PushNotifications/ChannelResult.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,15 @@ namespace winrt::Microsoft::ProjectReunion::implementation
{
ChannelResult() = default;

ChannelResult(Windows::Networking::PushNotifications::PushNotificationChannel const& channel, winrt::hresult const& extendedError, Microsoft::ProjectReunion::ChannelStatus const& status);
ChannelResult(Windows::Networking::PushNotifications::PushNotificationChannel const channel, winrt::hresult const extendedError, Microsoft::ProjectReunion::ChannelStatus const status);
Windows::Networking::PushNotifications::PushNotificationChannel Channel();
winrt::hresult ExtendedError();
Microsoft::ProjectReunion::ChannelStatus Status();

private:
Windows::Networking::PushNotifications::PushNotificationChannel m_channel{ nullptr };
winrt::hresult m_extendedError;
sharath2727 marked this conversation as resolved.
Show resolved Hide resolved
ChannelStatus m_channelStatus{};
};
}
namespace winrt::Microsoft::ProjectReunion::factory_implementation
Expand Down
88 changes: 87 additions & 1 deletion dev/PushNotifications/PushManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,97 @@
#include "pch.h"
#include "PushManager.h"
#include "PushManager.g.cpp"
#include <winrt\windows.networking.pushnotifications.h>
#include <winerror.h>
#include <algorithm>
#include <vector>
#include <iostream>
#include <ChannelResult.h>
#include <winerror.h>
#include <PushNotificationErrorCodes.h>

using namespace winrt::Windows::Networking::PushNotifications;

namespace winrt::Microsoft::ProjectReunion::implementation
{
std::vector<winrt::guid> PushManager::s_remoteIdList;
std::mutex PushManager::s_mutex;
std::unique_lock<std::mutex> PushManager::s_lock (s_mutex, std::defer_lock); // Avoid locking during the constructor call
inline constexpr std::uint32_t c_maxBackoffSeconds{ 960 };

bool PushManager::isChannelRequestRetryable(const winrt::hresult& hrException)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Rename to hr

There's nothing about this function that requires the parameter to have come from an exception.

{
switch (hrException)
sharath2727 marked this conversation as resolved.
Show resolved Hide resolved
{
case HRESULT_FROM_WIN32(ERROR_TIMEOUT):
case WnpErrors::WNP_E_NOT_CONNECTED:
case WPN_E_OUTSTANDING_CHANNEL_REQUEST:
sharath2727 marked this conversation as resolved.
Show resolved Hide resolved
case WnpErrors:: WNP_E_RECONNECTING:
case WnpErrors::WNP_E_BIND_USER_BUSY:
case RPC_S_SERVER_UNAVAILABLE:
return true;
default:
return false;
}
}

Windows::Foundation::IAsyncOperationWithProgress<Microsoft::ProjectReunion::ChannelResult, Microsoft::ProjectReunion::ChannelResult> PushManager::CreateChannelAsync(winrt::guid remoteId)
sharath2727 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we verify the implications of lifetime management? What would happen if the process exited but this AsyncProgress operation is not yet complete? Is the process termination graceful?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://en.cppreference.com/w/cpp/language/coroutines - When the coroutine state is destroyed either because it terminated via co_return or uncaught exception, or because it was destroyed via its handle, it does the following:

calls the destructor of the promise object.
calls the destructors of the function parameter copies.
calls operator delete to free the memory used by the coroutine state
transfers execution back to the caller/resumer.

So the coroutines completes gracefully.

Also it is important that the caller of the coroutine API: CreateChannelAsync will have to call IAsyncOperationWithProgress.Cancel to stop the asynchronous operation before the caller process exits. We would be clearly mentioning this in the sample that we are providing to cancel the asyncoperation if you are going out of scope. Thanks.

{
throw hresult_not_implemented();
winrt::Microsoft::ProjectReunion::ChannelResult channelResult{ nullptr };
sharath2727 marked this conversation as resolved.
Show resolved Hide resolved
auto progress{ co_await winrt::get_progress_token() };

winrt::check_pointer(&remoteId);

s_lock.lock();
sharath2727 marked this conversation as resolved.
Show resolved Hide resolved
if (std::find(s_remoteIdList.begin(), s_remoteIdList.end(), remoteId) != s_remoteIdList.end())
{
progress(winrt::make<winrt::Microsoft::ProjectReunion::implementation::ChannelResult>(nullptr, WPN_E_OUTSTANDING_CHANNEL_REQUEST, ChannelStatus::InProgress));
channelResult = winrt::make<winrt::Microsoft::ProjectReunion::implementation::ChannelResult>(nullptr, WPN_E_OUTSTANDING_CHANNEL_REQUEST, ChannelStatus::CompletedFailure);
co_return channelResult;
}
else
{
s_remoteIdList.push_back(remoteId);
}
s_lock.unlock();

PushNotificationChannelManager channelManager{};

for (auto backOffTimeInSeconds = 30; backOffTimeInSeconds <= c_maxBackoffSeconds * 2; backOffTimeInSeconds *= 2)
{
try
{
auto pushChannel = co_await channelManager.CreatePushNotificationChannelForApplicationAsync();

if (pushChannel != nullptr)
{
channelResult = winrt::make<winrt::Microsoft::ProjectReunion::implementation::ChannelResult>(pushChannel, S_OK, ChannelStatus::CompletedSuccess);
break;
}
}
catch (...)
{
auto ex = hresult_error(to_hresult(), take_ownership_from_abi);

if ((backOffTimeInSeconds <= c_maxBackoffSeconds) && isChannelRequestRetryable(ex.code()))
{
progress(winrt::make<winrt::Microsoft::ProjectReunion::implementation::ChannelResult>(nullptr, ex.code(), ChannelStatus::InProgressRetry));
}
else
{
channelResult = winrt::make<winrt::Microsoft::ProjectReunion::implementation::ChannelResult>(nullptr, ex.code(), ChannelStatus::CompletedFailure);
break;
}
}

co_await winrt::resume_after(std::chrono::seconds(backOffTimeInSeconds));
}

s_lock.lock();
std::remove(s_remoteIdList.begin(), s_remoteIdList.end(), remoteId);
s_lock.unlock();

co_return channelResult;

}
}
9 changes: 9 additions & 0 deletions dev/PushNotifications/PushManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
// Licensed under the MIT License. See LICENSE in the project root for license information.

#pragma once

#include "PushManager.g.h"
#include <vector>
#include <mutex>

namespace winrt::Microsoft::ProjectReunion::implementation
{
Expand All @@ -11,6 +14,12 @@ namespace winrt::Microsoft::ProjectReunion::implementation
PushManager() = default;

static Windows::Foundation::IAsyncOperationWithProgress<Microsoft::ProjectReunion::ChannelResult, Microsoft::ProjectReunion::ChannelResult> CreateChannelAsync(winrt::guid remoteId);

private:
static bool isChannelRequestRetryable(const winrt::hresult& hrException);
static std::vector<winrt::guid> s_remoteIdList;
static std::mutex s_mutex;
static std::unique_lock<std::mutex> s_lock;
};
}
namespace winrt::Microsoft::ProjectReunion::factory_implementation
Expand Down
14 changes: 14 additions & 0 deletions dev/PushNotifications/PushNotificationErrorCodes.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

#pragma once

namespace winrt::Microsoft::ProjectReunion::implementation
{
enum WnpErrors
{
WNP_E_NOT_CONNECTED = 0x880403E8L,
WNP_E_RECONNECTING = 0x880403E9L,
WNP_E_BIND_USER_BUSY = 0x880403FEL,
};
}
1 change: 1 addition & 0 deletions dev/PushNotifications/PushNotifications.vcxitems
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@
<ItemGroup>
<ClInclude Include="$(MSBuildThisFileDirectory)ChannelResult.h" />
<ClInclude Include="$(MSBuildThisFileDirectory)PushManager.h" />
<ClInclude Include="$(MSBuildThisFileDirectory)PushNotificationErrorCodes.h" />
</ItemGroup>
</Project>