> On Dec. 5, 2019, 10:38 a.m., Greg Mann wrote: > > 3rdparty/libprocess/src/ssl/socket_wrapper.hpp > > Lines 94 (patched) > > <https://reviews.apache.org/r/71666/diff/6/?file=2174527#file2174527line94> > > > > In this case, we don't really need an actor context, since there isn't > > any actor state associated with the compute thread. We really just want > > some context (any context) to dispatch the SSL-related functions onto, > > right? > > > > It would make a bit more sense to me to dispatch these functions > > without specifying an actor, so that libprocess can run them wherever it > > pleases. > > > > We could consider updating `loop()` to dispatch in all cases, even when > > no pid is specified. However, I do wonder if we're unknowingly depending on > > the existing behavior somewhere. In any case, changing loop to always > > `dispatch()` the iterate and body seems more desirable to me? > > > > However, the `loop()` calls below aren't strictly necessary I think. We > > could accomplish the same thing with dispatches and chained continuations, > > so we could also just use `dispatch()` directly instead of `loop()`, that > > might be the simplest thing to do. > > > > WDYT? > > Joseph Wu wrote: > I think a UPID/actor is required for any dispatching/looping on > libprocess worker threads, so this variable would still exist regardless of > how the loops are implemented. > > The alternative is to run everything on the event loop thread (or special > threads we spin up/acquire out of band?). > > Greg Mann wrote: > Ah I was remembering the version of `defer()` which can be invoked > without a pid: > https://github.com/apache/mesos/blob/925ad30c0f3b249afe74bdeb1921c5fdbf1c5886/3rdparty/libprocess/include/process/defer.hpp#L275-L283 > > Actually, I wish we had an overload of `dispatch()` that did something > similar. In any case, the `defer()` overload might work here, WDYT?
That overload of `defer()` ends up running things on a thread_local UPID: ``` // Per thread executor pointer. We use a pointer to lazily construct the // actual executor. extern thread_local Executor* _executor_; #define __executor__ \ (_executor_ == nullptr ? _executor_ = new Executor() : _executor_) ``` In this case, I think we would end up constructing a single `__executor__` object on the EventLoop thread (since that is where the `defer()` is called), so all socket IO would end up deferred onto the same UPID. - Joseph ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71666/#review218948 ----------------------------------------------------------- On Nov. 19, 2019, 4:29 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71666/ > ----------------------------------------------------------- > > (Updated Nov. 19, 2019, 4:29 p.m.) > > > Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Till > Toenshoff. > > > Bugs: MESOS-10010 > https://issues.apache.org/jira/browse/MESOS-10010 > > > Repository: mesos > > > Description > ------- > > This completes a fully functional client-side SSL socket. > > Needs a bit of cleanup and more error handling though. > > > Diffs > ----- > > 3rdparty/libprocess/src/ssl/socket_wrapper.hpp PRE-CREATION > 3rdparty/libprocess/src/ssl/socket_wrapper.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/71666/diff/6/ > > > Testing > ------- > > ``` > cmake --build . --target libprocess-tests > libprocess-tests > ``` > > Running libprocess-tests yields: > ``` > [ FAILED ] SSLTest.ValidDowngrade > [ FAILED ] SSLTest.ValidDowngradeEachProtocol > ``` > > > Thanks, > > Joseph Wu > >
