[ https://issues.apache.org/jira/browse/MESOS-5293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15299735#comment-15299735 ]
Abhishek Dasgupta commented on MESOS-5293: ------------------------------------------ Posted a RR on : https://reviews.apache.org/r/47822/ > Endpoint handlers for master and agent are implemented surprisingly > differently. > -------------------------------------------------------------------------------- > > Key: MESOS-5293 > URL: https://issues.apache.org/jira/browse/MESOS-5293 > Project: Mesos > Issue Type: Bug > Components: master, slave > Reporter: Benjamin Bannier > Assignee: Abhishek Dasgupta > Labels: tech-debt > > The way endpoints are routed is inconsistent between master and agent code > which makes adding or extending handlers error prone. > In the master we route like this: > {code} > // Setup HTTP routes. > route("/api/v1/scheduler", > DEFAULT_HTTP_FRAMEWORK_AUTHENTICATION_REALM, > Http::SCHEDULER_HELP(), > [this](const process::http::Request& request, > const Option<string>& principal) { > Http::log(request); > return http.scheduler(request, principal); > }); > {code} > We capture a pointer to the current {{Master}} in the callback which allows > us to access master state from an endpoint handler. The handler is a > (probably non-static) member function of the master's member variable > {{http}} which outlives the callback. > Routing in the agent looks like this: > {code} > // Setup HTTP routes. > Http http = Http(this); > route("/api/v1/executor", > Http::EXECUTOR_HELP(), > [http](const process::http::Request& request) { > Http::log(request); > return http.executor(request); > }); > {code} > In contrast to the master code we here copy a {{Http}} by value into the > callback. Since the callback is currently treated like a value and might > e.g., be freely copied around we are only guaranteed that {{http}} lives long > enough for the handler (here {{Http::executor}}) to return. In particular, > since endpoint handlers return a {{Future<Response>}} there is no guarantee > that the used {{http}} lives long enough to be still valid once a > conventional (e.g., non-static member) continuation is executed. > Both models have their merit: > * capturing a pointer to the master simplifies reasoning about lifetimes, > * capturing just a {{Http}} with very short lifetime minimizes interactions > among (e.g., concurrent) invocations of endpoint handlers. > This great inconsistency comes with a cost though, as employing patterns > borrowed from master endpoint handlers in agent code will lead to potentially > subtle bugs where a developer assuming that {{http}} would outlive a > handler's execution might introduce code invoking member functions of already > destructed variables. This is especially likely in code employing multiple > layers of {{delay}} or {{defer}} for which compilers seem unable to detect > lifetime problems and emit diagnostics. > It would be great if we could to use just one of the patterns to minimize > confusion, e.g., the more straight-forward master pattern. -- This message was sent by Atlassian JIRA (v6.3.4#6332)