merlimat commented on a change in pull request #12586:
URL: https://github.com/apache/pulsar/pull/12586#discussion_r741626030
##########
File path: pulsar-client-cpp/lib/Future.h
##########
@@ -99,21 +100,24 @@ class Promise {
}
state->value = value;
- state->result = Result();
+ state->result = DEFAULT_RESULT;
state->complete = true;
- typename std::list<ListenerCallback>::iterator it;
- for (it = state->listeners.begin(); it != state->listeners.end();
++it) {
- ListenerCallback& callback = *it;
- callback(state->result, state->value);
+ const auto listeners = std::move(state->listeners);
+
+ lock.unlock();
+
+ for (auto& callback : listeners) {
+ callback(DEFAULT_RESULT, value);
}
state->listeners.clear();
Review comment:
The only thing I’m not 100 sure if it’s safe to keep using the listener
vector after it's moved out. Eg. If a new listener is added after the future
is completed, it will add again into the moved vector which is in an undefined
state. A I understand, the only guarantee is that the vector will be empty, but
we have no idea whether we can add or not.
Instead of the move, it might actually be safer to do a swap with an empty
std::vector.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
