iceoryx icon indicating copy to clipboard operation
iceoryx copied to clipboard

Implement `cxx::Future` and `cxx::Promise`

Open mossmaurice opened this issue 2 years ago • 13 comments

Brief feature description

Automotive bindings like ara::com are using std::Future and std::Promise in their APIs. However, they throw exceptions which is not in line with the iceoryx coding guidelines.

Constraints

  1. no dynamic memory
  2. no exceptions
  3. use safe STL (non-throwing, no dynamic memory, ...) and iceoryx hoofs only for implementation

Goals

  1. Support standard future use case
    • create a promise p
    • create a future f from p
    • move p to another thread
    • f waits for p to either set a value or an exception/error
    • once p sets the value or error f unblocks
    • value can be retrieved from the future
  2. As close to STL syntax as possible
    • generic values and exceptions/errors
    • not completely possible due to constraints
  3. Acceptable performance
    • constraints lead to copies and (potentially) locking
    • this would not be the case with dynamic memory, but most likely cannot be avoided without
  4. Safeguard against misuse cases where possible
    • at least detect them at runtime, e.g. broken promise
    • how to communicate a misuse error (terminate?)

Valid use

  1. p sets value, f wakes up and is destroyed before p
  2. p never creates a future (unusual but OK)

Invalid use

  1. create future (and wait) but destroy promise before setting a value or error (broken promise)
  2. p creates multiple futures
  3. create future from promise p, move p to another thread, move future
  4. set the value or error multiple times at the promise
  5. create another future after setting value (could be supported if it is deemed worth it)

Limitations

  1. Move semantics are limited (no dynamic memory)
    • promise can be moved
    • future cannot be moved once the promise may set a value
    • there is at most one valid (active/waiting) future for each promise
  2. As in STL, futures and promises are not copyable
    • shared_future can be an extension (multiple futures wait for some promise)

Implementation considerations

  • in STL future and promise have a shared state
  • shared state is allocated dynamically
  • in icoeryx the shared state can exist in promise or future on the stack
  • lifetime of this state is crucial (as long as a promise or future exists it has to point to a valid state)
  • only one of them holds the state at any time
  • initially the promise holds the state
  • as soon as the promise creates a future, the state is moved to this future (hence the future must outlive the promise or the state moved back)
  • moving the state may require locking (this may create very subtle races)
  • a semaphore is part of the state to wait for the result/error
  • the state contains the value, i.e. its size depends on the generic type parameter of the promise/future
  • once the future was set it is not supposed to be set again, hence the future with its state may go out of scope (but needs to inform the promise to prevent accessing the state)

Open Issues

Refers to prototype implementation in https://github.com/MatthiasKillat/iceoryx/tree/iox-futures-experimental

  • clean up code
  • testing (API, use cases and error cases where possible)
  • proper definition of all misuse cases
  • analysis of potential race conditions and fixes (or stated as further limitation)
  • optimize state locking (may not be needed in some cases)
  • remove exceptions (exist temporary, thrown in misuse cases)
  • API changes to communicate errors (as we cannot use exceptions, use expected<T,E>?)
  • limits the error type to one E (can be circumvented with error inheritance hierarchy if needed)
  • optional syntactic sugar, e.g. std::packaged_task

Detailed information

  • [x] Analyse feasibility of such an implementation
    • [ ] Check how API could be as close to the STL but with using cxx::expected
  • [ ] Implement exception-free, stack-based cxx::Future
  • [ ] Implement exception-free, stack-based cxx::Promise

mossmaurice avatar Jun 10 '22 08:06 mossmaurice

@mossmaurice See https://github.com/MatthiasKillat/iceoryx/tree/iox-futures-experimental for an incomplete approach how to implement std::future if this is the actual problem.

Now there are a lot of subtle races that need to be solved when attempting an implementation, especially if no dynamic memory is allowed.

Furthermore ara::future is maybe allowed to behave entirely differently or expected to perform more functionality, like communication with a server in another process (this is naturally not the case for std::future). See https://github.com/eclipse-iceoryx/iceoryx/discussions/1389#discussioncomment-2923045 for some additional explanation.

MatthiasKillat avatar Jun 10 '22 13:06 MatthiasKillat

@MatthiasKillat : Can we have short meeting regarding this sometime next week?

hemalbavishi avatar Jul 22 '22 12:07 hemalbavishi

@hemalbavishi This week I am almost completely unavailable. We can do this maybe on Friday this week at the earliest (if it is short and requires almost no preparation). Sometime next week (e.g. Monday) would be preferable though. If Monday or later I could prepare more information/code.

We could use a zoom meeting which I would announce in https://github.com/eclipse-iceoryx/iceoryx/wiki/Deep-Dive-Sessions. I would need some preferred time frame to set this up (e.g. Monday afternoon CEST).

Let me know whether this works and the preferred time (not earlier than next week).

MatthiasKillat avatar Jul 25 '22 09:07 MatthiasKillat

@MatthiasKillat : It sounds good. Let's have a call next week to discuss more in detail. My colleague has integrated code provided by you but we would like to understand race conditions to make it better. Let's discuss more in detail on Monday - Aug. 01

hemalbavishi avatar Jul 26 '22 10:07 hemalbavishi

@hemalbavishi I will set up a zoom meeting and ping you again. Note that the existing code was not error free at all. The plan was to improve it and thoroughly test it. But we can discuss this in the call.

Edit: see comment below for a meeting proposal. .

MatthiasKillat avatar Jul 26 '22 10:07 MatthiasKillat

@hemalbavishi After some discussion with my colleagues we considered that it would be better to discuss this in the next iceoryx developer meetup on August 4th (i.e. next Thursday, see https://calendar.google.com/calendar/u/0/[email protected]&ctz=America/Los_Angeles) Would this still work?

At least for me this would be easier to rethink the issue again and prepare something (maybe even clean up some experimental code over the weekend).

The goal would be to discuss your use case, potential solutions and the concurrency problems I have already encountered if we try to solve this without dynamic memory (if not, it is as simple as using std::promise). Or do we require a more specific promise for the use case?

Please let me know whether the iceoryx developer meetup works as well.

MatthiasKillat avatar Jul 26 '22 12:07 MatthiasKillat

@MatthiasKillat : Okay. I will try to share some points with you before the call. It would be great if you can also do the same after your work over weekend.

hemalbavishi avatar Jul 27 '22 11:07 hemalbavishi

@hemalbavishi Yes, I can provide more information in the iceoryx developer meetup.

MatthiasKillat avatar Jul 27 '22 13:07 MatthiasKillat

@MatthiasKillat : Please update the ticket with open issues for future and promise implementation.

SomnathHolkar avatar Aug 17 '22 11:08 SomnathHolkar

@SomnathHolkar Just a short FYI: @MatthiasKillat is currently on vacation and comes back in CW35.

mossmaurice avatar Aug 17 '22 12:08 mossmaurice

@mossmaurice @SomnathHolkar I updated the issue with the use and misuse cases and implementation considerations. This reflects the current state of implementation, which likely has subtle races (which can be fixed once understood). I will improve the implementation once I have time but I cannot commit to this in the next week (due to vacation).

MatthiasKillat avatar Aug 18 '22 13:08 MatthiasKillat

Hello Matthias,

We didn't get much time to work on that. Do you have any update from our last discussion? We want to take this forward and have std::promise and std::future heap free.

Thank you!

hemalbavishi avatar Jan 16 '23 06:01 hemalbavishi

@hemalbavishi We did not work on it either. I will look into it in the coming days.

MatthiasKillat avatar Jan 16 '23 09:01 MatthiasKillat