CPP-987 Make handling of future callbacks play nicely with the predicate guards around cass_future_wait #559
Conversation
|
Reviewed manually with @joao-r-reis and @hhughes. Primary take away was clearer documentation on testing regime, which I've subsequently provided in a comment on the original JIRA ticket. Jenkins failures are due to an unrelated issue. |
| // CPP-987 Set this after the callbacks run to avoid unexpected exits | ||
| // from wait ops due to spurious wakeups | ||
| is_set_ = true; | ||
|
|
There was a problem hiding this comment.
Note: this will get mentioned in the release notes for the corresponding release as a possible change in behaviour for callbacks. In reality it shouldn't result in much of a change at all; it might change behaviours for very aberrant cases (like, say, calling cass_future_wait() in the callback) but any such behaviours are likely undocumented (and almost certainly wrong).
There was a problem hiding this comment.
Won't this also prevent calling cass_future_get_result in the callback? Is there a problem with that otherwise? Or another recommended way to handle processing the results without blocking the call that invoked the query?
There was a problem hiding this comment.
Exactly, We are facing the same issue, as in our case the async processing expects that cb_fn() i.e. callback function should be able to call cass_future_error_code() or cass_future_get_result() and process the results for further action.
Rather, please try to run build/examples/callbacks with this change,
We can see a hang there itself:
(gdb) bt
#0 futex_wait_cancelable (private=0, expected=0, futex_word=0x61ee58) at ../sysdeps/unix/sysv/linux/futex-internal.h:88
#1 __pthread_cond_wait_common (abstime=0x0, mutex=0x61ee00, cond=0x61ee30) at pthread_cond_wait.c:502
#2 __pthread_cond_wait (cond=0x61ee30, mutex=0x61ee00) at pthread_cond_wait.c:655
#3 0x00007ffff718421d in uv_cond_wait (cond=<optimized out>, mutex=<optimized out>) at src/unix/thread.c:780
#4 0x00007ffff782bf6e in datastax::internal::core::Future::internal_wait(datastax::internal::ScopedLock<datastax::internal::Mutex>&) () from /root//cpp-driver/build/libcassandra.so.2
#5 0x00007ffff782beea in datastax::internal::core::Future::error() () from /root//cpp-driver/build/libcassandra.so.2
#6 0x00007ffff782b87c in cass_future_error_code () from /root//cpp-driver/build/libcassandra.so.2
#7 0x0000000000401973 in on_session_connect ()
#8 0x00007ffff782bd9f in datastax::internal::core::Future::internal_set(datastax::internal::ScopedLock<datastax::internal::Mutex>&) () from /root//cpp-driver/build/libcassandra.so.2
#9 0x00007ffff7898bbc in datastax::internal::core::Future::set() () from /root//cpp-driver/build/libcassandra.so.2
#10 0x00007ffff7897ecd in datastax::internal::core::SessionBase::notify_connected() () from /root//cpp-driver/build/libcassandra.so.2
#11 0x00007ffff7894c6c in datastax::internal::core::SessionInitializer::on_initialize(datastax::internal::core::RequestProcessorInitializer*) () from /root//cpp-driver/build/libcassandra.so.2
#12 0x00007ffff7896fe5 in datastax::internal::Callback<void, datastax::internal::core::RequestProcessorInitializer*>::MemberInvoker<void (datastax::internal::core::SessionInitializer::*)(datastax::internal::core::RequestProcessorInitializer*), datastax::internal::core::SessionInitializer>::invoke(datastax::internal::core::RequestProcessorInitializer* const&) const () from /root//cpp-driver/build/libcassandra.so.2
#13 0x00007ffff788551e in datastax::internal::Callback<void, datastax::internal::core::RequestProcessorInitializer*>::operator()(datastax::internal::core::RequestProcessorInitializer* const&) const ()
from /root//cpp-driver/build/libcassandra.so.2
#14 0x00007ffff7884c7a in datastax::internal::core::RequestProcessorInitializer::on_initialize(datastax::internal::core::ConnectionPoolManagerInitializer*) () from /root//cpp-driver/build/libcassandra.so.2
There was a problem hiding this comment.
There was a problem hiding this comment.
Based on a quick review it very much looks like the analysis above is correct. I strongly suspect we'll have to revert the prior fix and figure out another way to handle CPP-987.
There was a problem hiding this comment.
To follow up here: the changes in CPP-987 have been reverted. I'm going to start working on finding another way to resolve those issues.
absurdfarce
changed the title
3 participants
No description provided.