style

OKWS Style, Convention and Coding Tips

Most of the work in an OKWS service is done inside a class that inherits from okclnt_t. Assume a trivial class derived from okclnt_t and also a related class direved from oksrvc_t:

  class oksrvc_foo_t : public oksrvc_t {
  public:
    oksrvc_foo_t (int argc, char *argv[]) : oksrvc_t (argc, argv) {}
    okclnt_t *make_newclnt (ptr<ahttpcon> x);
  };

  class okclnt_foo_t : public okclnt_t {
  public:
    okclnt_foo_t (ptr<ahttpcon> x, oksrvc_foo_t *v)
      : okclnt_t (x, o), _foo_service (v), _i (11) {}
    ~okclnt_foo_t () {}
    void process () { process_impl (); }
  private:
    void process_impl (CLOSURE);
    void helper_routine (cbb cb, CLOSURE);
    void format_output (cbv cb, CLOSURE);
    void collect_data (cbb cb, CLOSURE);
    void verify_data (cbb cb, CLOSURE);
    oksrvc_foo_t *_foo_service;
    int _i;
  };

That is, this is a service foo, implemented with an okclnt_foo_t object per HTTP connection, and a single long-lived oksrvc_foo_t object that lives for the lifetime of the Unix process. This is the basic scaffolding. The point of this document is to discuss some of the finer points of implementing the okclnt_t derived class in particular.

How To Implement a ::process member method

The okclnt_t::process method is pure virtual. Hence, any class inheriting from okclnt_t must implement ::process. As shown above, the easiest was to do this is via tame. Here, a helper function okclnt_foo_t::process_impl is where all the action is, and the standard okclnt_foo_t::process method just calls this implementation. The reason for this indirection is that sometimes the tame system produces unexpected results when implementing a virtual function. This case is probably fine, since there is no underlying implementation of ::process that's being overloaded, but this particular tame-workaround is a good habit, and can never hurt.

Drilling down a bit further, the implementation in question might look something like this:

tamed void
okclnt_foo_t::process_impl ()
{
  tvars {
    bool ok;
    str redir_loc ("/error_page");
  }
  
  // body
  twait { collect_data (mkevent (ok)); }
  if (ok) {
    twait { verify_data (mkevent ok)); }
    if (!ok) {
      redir_loc = "/bad_data";
    } else {
       format_output (mkevent ());
    }
  }

  // epilogue
  if (!ok) {
    redirect (redir_loc);
  } else { 
    output (out);
  }

  // XXX don't access _i after calling output or redirect!
  // _i = 10;
  // XXX
}

There are several important details to notice in this example, that can be generalized:

• Either okclnt_t::output is called or okclnt_t::redirect is called. In the above example, the redirect method is called in the case of error, and the output method is called if the request was successfully completed. The out buffer belongs to the okclnt_t object and was filled in with format_output.
• No members of okclnt_foo_t are accessed after a call to redirect or output. Both of these functions call delete this;, so accessing this->_i after calling output will reference free memory and can cause a coredump.
• ** Either okclnt_t::output or okclnt_t::redirect is called exactly once, and the other not at all.** Because these methods call delete this; they obviously cannot be called more than once per okclnt_t object.

The easiest was to guarantee these invariants is to break your process_impl function into two parts:

• a body in which the input parameters are processed, the relevant database calls are made, and the output formatted
• an epilogue in which the page is outputted or redirected.

Others Tips for Tamed Functions

Consider a dummy implementation of a method like okclnt_foo_t::verify_data, whose goal is to read in CGI variables from the user and to determine if they constitute a valid request:

tamed void
okclnt_foo_t::verify_data (cbb cb)
{
  tvars {
    // variables here
  }
  if ( !check1 () ) {
    (*cb) (false);
    return;
  }
  if (!check2 ()) {
    (*cb) (false);
    return;
  }
  // otherwise, OK!
  (*cb) (true);
}

Obviously, this example is contrived, but note the following important points:

• The function verify_data "returns" its results back to its caller via its callback. It supplies true as a parameter if the verification succeeded, and false otherwise.
• In every path through the function, the callback is called exactly once. This is crucial; if the callback were called 0 times, then the caller function (okclnt_foo_t::process_impl) would hang indefinitely waiting for verify_data to complete. If the callback were called twice, the second call might result in unexpected writes to memory and could crash the program. tame has built-in protection against this bug, and the runtime will panic when it's detected.

In terms of style, the following equivalent way to write the function is less error-prone, and easier to follow:

tamed void
okclnt_foo_t::verify_data (cbb cb)
{
  tvars {
    // variables here
    bool ret (true);
  }
  if ( !check1 () ) {
    ret = false;
  } else if (!check2 ()) {
    ret = false;
  }
  // otherwise, OK!
  (*cb) (ret);
}

As in okclnt_foo_t::process_impl above, we're trying to make only one exit point from the method -- when control runs off the end of the function. This point in the code is a convenient time for verify_data to call its callback, since it will happen only once per invocation. Also note that verify_data's control flow is easy to follow, without any short-curcuit return logic. Such code layout makes debugging simpler, and code less error-prone. The downside is that it can result is very deeply nested functions.

A final alternative for writing such a function is with a tame feature called autocb. The idea behind autocb is to call a given callback when the autocb object goes out of scope. OkCupid has a version of autocb called a trigger class. The code can be used as follows:

#include "autocb.h"

tamed void
okclnt_foo_t::verify_data (cbb cb)
{
  tvars {
    // variables here
    bool ret (true);
    holdvar autocb_t<bool> acb (cb, ret);
  }
  if ( !check1 () ) {
    ret = false;
    return;
  }
  if (!check2 ()) {
    ret = false;
    return;
  }
  // otherwise, OK!
  ret = true;
}

The given autocb object will stay in scope as long as verify_data's closure is in scope. Once it goes out of scope, acb will also be destroyed and cb will be called with the value of ret at the time of the deeallocation.

The keyword holdvar is a hint picked up by the tame translator. It instructs the translator not to make a reference to acb within the body of verify_data. However, room for acb is still allocated with the closure, and acb will therefore share fate with the closure.