Skip to content

Commit

Permalink
work
Browse files Browse the repository at this point in the history
  • Loading branch information
tcormackMW committed Jul 9, 2024
1 parent 182ed06 commit 80022bc
Show file tree
Hide file tree
Showing 3 changed files with 315 additions and 0 deletions.
275 changes: 275 additions & 0 deletions text/0014-Safe-Future.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,275 @@
- Start Date: 2024-07-01
- RFC PR: https://github.com/CppMicroServices/CppMicroServices/commit/

# Safe Futures

## Summary

The SafeFuture object allows for the following:

**SafeFuture for Configuration Updates**: The new `SafeFuture` feature is returned from calls to update configurations. This allows callers, who are allocated in threads that are allocated on the AsyncWorkService, to wait on configuration updates without risking deadlocks in the threadpool.

## Motivation
Various CppMicroServices require external configuration. These configurations are created using the ConfigAdmin service, and calls to `Update`, `UpdateIfDifferent`, and `Remove`. However, currently if those calls are made from a thread allocated on the AsyncWorkService, there is a possibility of overwhelming that threadpool and deadlocking.

The introduction of the `SafeFuture` feature enhances this solution by providing a safe and efficient way to handle asynchronous configuration updates. This ensures that services can wait for updates without risking deadlocks, thereby improving the robustness and reliability of the configuration management process.

## Detailed design
### Functional Design

Rather than returning a raw future to the user that they use to directly access the configuration update task, we wrap it with logic that checks for suspected deadlocks and resolves them if present.

**SafeFuture.h**
```c++
/*=============================================================================
Library: CppMicroServices
Copyright Kevlin Henney, 2000, 2001, 2002. All rights reserved.
Extracted from Boost 1.46.1 and adapted for CppMicroServices.
Permission is hereby granted, free of charge, to any person or organization
obtaining a copy of the software and accompanying documentation covered by
this license (the "Software") to use, reproduce, display, distribute,
execute, and transmit the Software, and to prepare derivative works of the
Software, and to permit third-parties to whom the Software is furnished to
do so, all subject to the following:
The copyright notices in the Software and this entire statement, including
the above license grant, this restriction and the following disclaimer,
must be included in all copies of the Software, in whole or in part, and
all derivative works of the Software, unless such copies or derivative
works are solely in the form of machine-executable object code generated by
a source language processor.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE, TITLE AND NON-INFRINGEMENT. IN NO EVENT
SHALL THE COPYRIGHT HOLDERS OR ANYONE DISTRIBUTING THE SOFTWARE BE LIABLE
FOR ANY DAMAGES OR OTHER LIABILITY, WHETHER IN CONTRACT, TORT OR OTHERWISE,
ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS IN THE SOFTWARE.
=========================================================================*/

#ifndef CPPMICROSERVICES_SAFEFUTURE_H
#define CPPMICROSERVICES_SAFEFUTURE_H

#include "cppmicroservices/BundleContext.h"
#include "cppmicroservices/Framework.h"
#include "cppmicroservices/detail/ScopeGuard.h"

#include <future>
#include <vector>

namespace cppmicroservices
{
using ActualTask = std::packaged_task<void(bool)>;
using PostTask = std::packaged_task<void()>;
class US_Framework_EXPORT SafeFuture
{
public:
SafeFuture(std::shared_future<void> future,
std::shared_ptr<std::atomic<bool>> asyncStarted = nullptr,
std::shared_ptr<ActualTask> task = nullptr);
SafeFuture() = default;

// Destructor
~SafeFuture() = default;

// Copy constructor
SafeFuture(SafeFuture const& other) = default;

// Copy assignment operator
SafeFuture& operator=(SafeFuture const& other) = default;

// Move constructor
SafeFuture(SafeFuture&& other) noexcept = default;

// Move assignment operator
SafeFuture& operator=(SafeFuture&& other) noexcept = default;

// Method to get the result
void get() const;
void wait() const;
template <class Rep, class Period>
std::future_status
wait_for(std::chrono::duration<Rep, Period> const& timeout_duration) const
{
return future.wait_for(timeout_duration);
}

std::shared_future<void> retrieveFuture(){
return future;
}

private:
std::shared_future<void> future;
std::shared_ptr<std::atomic<bool>> asyncStarted;
std::shared_ptr<ActualTask> task;
};
} // namespace cppmicroservices

#endif // CPPMICROSERVICES_SAFEFUTURE_H
```

**Changes to ConfigurationImpl.hpp**
```c++
namespace cppmicroservices::cmimpl
{
/**
* Update the properties of this Configuration.
*
* @return a std::shared_future<void>
* @note not safe to wait on future from within the AsyncWorkService
*
* See {@code Configuration#Update}
*/
std::shared_future<void> Update(AnyMap properties) override;

/**
* Update the properties of this Configuration and return
*
* @return A SafeFuture, safe to wait on from within the
* AsyncWorkService used by the Framework
*
* See {@code Configuration#Update}
*/
SafeFuture SafeUpdate(AnyMap newProperties) override;

/**
* Update the properties of this Configuration if they differ from the current properties.
*
* @return a std::shared_future<void> and whether the config was updated
* @note not safe to wait on future from within the AsyncWorkService
*
* See {@code Configuration#UpdateIfDifferent}
*/
std::pair<bool, std::shared_future<void>> UpdateIfDifferent(AnyMap properties) override;

/**
* Update the properties of this Configuration if they differ from the current properties.
*
* @return A SafeFuture, safe to wait on from within the
* AsyncWorkService used by the Framework
*
* See {@code Configuration#UpdateIfDifferent}
*/
std::pair<bool, SafeFuture> SafeUpdateIfDifferent(AnyMap properties) override;

/**
* Remove this Configuration from ConfigurationAdmin.
*
* @return A std::shared_future<void>,
* @note not safe to wait on future from within the AsyncWorkService
*
* See {@code Configuration#Remove}
*/
std::shared_future<void> Remove() override;

/**
* Remove this Configuration from ConfigurationAdmin.
*
* @return A SafeFuture, safe to wait on from within the
* AsyncWorkService used by the Framework
*
* See {@code Configuration#Remove}
*/
SafeFuture SafeRemove() override;
}
```
**Changes to ConfigurationAdminPrivate.hpp**
```c++
/**
* Internal method used to notify any {@code ManagedService} or {@code ManagedServiceFactory} of an
* update to a {@code Configuration}. Performs the notifications asynchronously with the latest state
* of the properties at the time.
*
* @param pid The PID of the {@code Configuration} which has been updated
*/
virtual SafeFuture NotifyConfigurationUpdated(
std::string const& pid,
unsigned long const changeCount)
= 0;
/**
* Internal method used by {@code ConfigurationImpl} to notify any {@code ManagedService} or
* {@code ManagedServiceFactory} of the removal of a {@code Configuration}. Performs the notifications
* asynchronously.
*
* @param pid The PID of the {@code Configuration} which has been removed.
* @param configurationId The unique id of the configuration which has been removed. Used to avoid race
* conditions.
*/
virtual SafeFuture NotifyConfigurationRemoved(
std::string const& pid,
std::uintptr_t configurationId,
unsigned long changeCount)
= 0;
```

**Changes to ConfigurationAdminImpl.hpp**
```c++
/**
*
* See {@code ConfigurationAdminPrivate#NotifyConfigurationUpdated}
*/
SafeFuture NotifyConfigurationUpdated(std::string const& pid, unsigned long const changeCount) override;

/**
* Internal method used by {@code ConfigurationImpl} to notify any {@code ManagedService} or
*
* See {@code ConfigurationAdminPrivate#NotifyConfigurationRemoved}
*/
SafeFuture NotifyConfigurationRemoved(std::string const& pid,
std::uintptr_t configurationId,
unsigned long changeCount) override;

```
**Changes to Configuration.hpp**
```c++
/**
* Same as Update() except:
* @return a future that is safe to wait on from within a thread allocated to the AsyncWorkService
*/
virtual SafeFuture SafeUpdate(AnyMap properties = AnyMap { AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS }) = 0;
/**
* Same as UpdateIfDifferent() except:
* @return a future that is safe to wait on from within a thread allocated to the AsyncWorkService
*/
virtual std::pair<bool, SafeFuture> SafeUpdateIfDifferent(
AnyMap properties = AnyMap { AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS })
= 0;
/**
* Same as Remove() except:
* @return a future that is safe to wait on from within a thread allocated to the AsyncWorkService
*/
virtual SafeFuture SafeRemove() = 0;
```

### Non-Functional Requirements
- Support asynchronous configuration updates with either method, `Update`, `UpdateIfDifferent`, and `Remove` or the `Safe` version of each.
` Support threadsafe synchronous waiting when using a discrete threadpool for the above 3 operations

### Architecturally Significant Design Case
#### User thread, allocated on a AsyncWorkService thread, calls Update on a configuration
From a thread allocated to the Threadpool, a client will call `Update()` on a configuration. This update will be posted to the threadpool. In the simplest case of a threadpool size 1, and the old `Update()` API, this will deadlock as shown below.

<html>

<img src="0014-safe-future/unsafeUpdate.puml" style="width:700px"/>

</html>

However with the new `Safe` API, this is resolved as follows:

<html>

<img src="0014-safe-future/safeUpdate.puml" style="width:700px"/>

</html>
20 changes: 20 additions & 0 deletions text/0014-safe-future/safeUpdate.puml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
@startuml addbundle_diagram

rectangle "BundleEvent" as BE
rectangle "BundleListener" as BL
'rectangle "BundleTracker" as BT
'rectangle "BundleTrackerPrivate" as BTP
rectangle "TrackedBundle" as TB
'rectangle "BundleContext" as BC
rectangle "BundleAbstractTracked" as BAT
rectangle "BundleTrackerCustomizer" as BTC

BE -> BL : 1: Passed into listener
BL -> TB : 2: BundleChanged()
TB -> BAT : 3: Track()
TB <- BAT : 4: CustomizerAdding()
TB -d-> BTC : 5: AddingBundle()

note bottom of BAT : Bundle not in map\nEntering state mask\nIncrement count

@enduml
20 changes: 20 additions & 0 deletions text/0014-safe-future/unsafeUpdate.puml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
@startuml addbundle_diagram

rectangle "BundleEvent" as BE
rectangle "BundleListener" as BL
'rectangle "BundleTracker" as BT
'rectangle "BundleTrackerPrivate" as BTP
rectangle "TrackedBundle" as TB
'rectangle "BundleContext" as BC
rectangle "BundleAbstractTracked" as BAT
rectangle "BundleTrackerCustomizer" as BTC

BE -> BL : 1: Passed into listener
BL -> TB : 2: BundleChanged()
TB -> BAT : 3: Track()
TB <- BAT : 4: CustomizerAdding()
TB -d-> BTC : 5: AddingBundle()

note bottom of BAT : Bundle not in map\nEntering state mask\nIncrement count

@enduml

0 comments on commit 80022bc

Please sign in to comment.