Friday 12 October 2012

Re-re-re-etc-design

Progress has slowed down a bit. First, because work shifted into overdrive, with impact on my free time. Second, because of this, namely: "Designing an interface takes longer than whipping together a concrete class which fulfills that interface". Getting libbsh2 and boost::asio to work was, it turns out, the easy part. Creating a robust class out of this is turning out to be a bit more difficult.

This is the 5th time I've started writing this post. Every time I begin, I find myself having to think deeper about my work (so I can explain it properly), which leads me to an insight, usually followed by some testing, a redesign, and some more testing. Then, I let it settle for a while, and start writing a post about it. Rinse and repeat.

So...

After reviewing SSHSession vis-à-vis items from Scott Meyer's "Effective C++" and "More Effective C++", I've noticed some problems to correct, which led to a redesign; which led to more problems; which was useful to remind me that, despite all the progress I've made, I'm still a beginner (let's just say I've created more than my share of source files with names like "test_*.cpp" in these last couple of weeks).

The main goals behind the redesign are:
  • Simplifying SSHSession's ctor and dtor, by implementing RAII on these resources: socket, session instance (resources created by libssh2 on the local machine), and session connection (actual ssh session established on the remote host).
  • Making SSHSession more robust to failure.

 The life cycle of a libssh2 session goes through these stages:
  1. Initialization (acquires local resources)
  2. Handshake/Authentication (establishes an ssh session on the remote host)
  3. Disconnect (terminates ssh session on the remote host)
  4. Free (releases local resources; if necessary, also performs step 3).
Before step 1, we have to open a socket, which we pass to libssh2, in step 2. From then on, we don't need to pass the socket anymore, as libssh2 will store a reference to it. After step 4 (and not before), we can close the socket.

So, after a few iterations, I arrived at this:
  • LoopAsync() and LoopTimer() (see here) will be free functions. Each will have a new parameter added - a socket/timer reference, upon which they'll act.
  • SSHSession will have a nested class, SessionConnection. This class is responsible for ssh handshake/authentication (on ctor) and ssh disconnect (on dtor), thus implementing RAII for the session connection (steps 2 and 3, above).
  • SessionConnection will have a nested class SessionHandle, that will encapsulate a LIBSSH2_SESSION *. This class is responsible for calling libssh2 functions that acquire (in its ctor) and free (in its dtor) local resources (steps 1 and 4, above).
I'm still on the fence about how to best handle the socket. I'm considering two alternatives:
  • SessionConnection will also be responsible for the socket, creating it on its ctor and destroying in on its dtor. This causes an order dependency on the dtor - the socket must remain open until SessionHandle dtor exits, as LIBSSH2_SESSION attempts a cleanup on the remote server, if necessary, in the case of an abnormal termination.
OR
  • SessionHandle will contain the socket, since it may need it in the dtor, as LIBSSH2_SESSION attempts a cleanup on the remote server, if necessary. This will probably mean a class rename, since "Handle" implies something simpler, such as a thin wrapper around a resource.
Right now, I'm more inclined towards the first alternative, in spite of the order dependency.

Both cases will require a method to create the socket, as asio's tcp::socket has no ctor that accepts a host name, only IP address (via tcp::endpoint). So, whoever end up owning the socket will also have a function that creates it and returns it by std::move.

No comments:

Post a Comment