Friday, 15 February 2013

Qt upgrade

I've just installed Qt Creator 2.6.2 and Qt 5.

Right now, I have:
  • Qt Creator 2.5 + Qt 4.8 using mingw/gcc 4.7.2 (let's call it "my mingw"); and
  • Qt Creator 2.6.2 + Qt 5 using mingw/gcc 4.7.0 (let's call it "Qt Creator's mingw", since this is the mingw installed with Qt Creator 2.6.2).
My goal is to have Qt Creator 2.6.2 + Qt 5 using my mingw. But right now, I'm testing the default Qt 5 env with my existing libs, to make sure I understand what's going on during linking, especially concerning mingw's std libs.

For these tests, I'm using a new concept introduced in Qt Creator, called "Kit". Each kit defines an environment (compiler, debugger, and Qt version, among other options), and once you create a kit, you can then switch to Projects mode to add it and configure its build/run settings.

E.g., I can have a kit with Qt Creator's mingw and Qt 5, another kit with my mingw and Qt5, and another kit with my mingw and Qt 4.8 (my current stable env). Then, in Projects mode, I select a kit and build. Each kit builds to its own folder, which is perfect for this kind of tests.

I did find a couple of oddities:
  • The default kit used my mingw, instead of using Qt Creator's mingw.
  • I cloned the default kit and changed it to use Qt Creator's mingw. By default, the build config was Release. I changed it to Debug and built; and it placed the binaries on the Release folder. Then, I changed it back to Release and rebuilt; and it placed the binaries on the Debug folder. Bug?

Saturday, 9 February 2013

Let's keep moving (will the puns ever end?)

So, how is my first Adventure in Moveland going?

The solution for my classes went according to planned, which was nice. SessionHandle, being the class that holds LIBSSH2_SESSION*, found itself suddenly surrounded by many new friends:

friend class SessionConnection;
friend class RemoteSessionHandle;
friend class SessionAuthenticator;
friend class RemoteChannelHandle;

SessionHandle::GetSession(), which provides access to said LIBSSH2_SESSION*, went from public to private. And both SessionHandle and RemoteSessionHandle (which also gained SessionConnection as friend) now have private Reseat() functions, to allow SessionConnection to reseat the pointers on move:

SessionConnection::SessionConnection(SessionConnection&& other) :
    reportStatus(move(other.reportStatus)), host(move(, 
    port(move(other.port)), user(move(other.user)), pwd(move(other.pwd)), 
    ios(move(other.ios)), sock(move(other.sock)), 
    remoteSessionHandle.Reseat(reportStatus, sock, sessionHandle);

So, everything went smoothly, right? Well, not quite. When I tested my move ctor, I was still getting a SegFault. It was now happening during the socket's dtor, more exactly here (boost's scoped_lock.hpp):

// Constructor acquires the lock.
scoped_lock(Mutex& m)
  : mutex_(m)

So, it looked like I was still going to learn something more about Boost.Asio. I began tracing the socket's dtor. Here's boost::asio::basic_stream_socket's definition:

template <typename Protocol,
    typename StreamSocketService = stream_socket_service<Protocol> >
class basic_stream_socket
  : public basic_socket<Protocol, StreamSocketService>

Notice the stream_socket_service<>? Let's take a look at it (relevant info only):

template <typename Protocol>
class stream_socket_service
  : public boost::asio::io_service::service
  : public boost::asio::detail::service_base<stream_socket_service<Protocol> >

  // The type of the platform-specific implementation.
#if defined(BOOST_ASIO_HAS_IOCP)
  typedef detail::win_iocp_socket_service<Protocol> service_impl_type;
  typedef detail::reactive_socket_service<Protocol> service_impl_type;
  // The platform-specific implementation.
  service_impl_type service_impl_;

Since we're on Windows and have BOOST_ASIO_HAS_IOCP #defined, let's take a look at win_iocp_socket_service:

template <typename Protocol> 
class win_iocp_socket_service : public win_iocp_socket_service_base

There is actually little of interest here, so we move on to win_iocp_socket_service_base:

class win_iocp_socket_service_base
// Mutex to protect access to the linked list of implementations.
boost::asio::detail::mutex mutex_;


Holy Shmoly, Duck Dodgers! It has a mutex! And it's neither a reference nor a pointer!

So, what does it hold? In our case (Windows), it's an instance of boost::asio::detail::win_mutex, and it encapsulates an OS critical section object. And it performs RAII, too; win_mutex's dtor looks like this:


We can read here what DeleteCriticalSection() does, but this is the important bit:

After a critical section object has been deleted, do not reference the object in any function that operates on critical sections (such as EnterCriticalSection, TryEnterCriticalSection, and LeaveCriticalSection) other than InitializeCriticalSection and InitializeCriticalSectionAndSpinCount. If you attempt to do so, memory corruption and other unexpected errors can occur.

Memory corruption and other unexpected errors? Check, eager young space cadet.

So, basically, even though Boost.Asio's socket is moveable (according to std::is_move_constructible<> and std::is_move_assignable<>), it may hold objects that aren't. In this case, we were left with two instances of win_iocp_socket_service_base, each with its instance of win_mutex that pointed to the same OS critical section object.

When the first socket ("moved to") is destroyed, ~win_mutex() is called, and the OS is told to get rid of that pesky critical section object pointed by crit_section_. Which it dutifully does.

Then, the second socket ("moved from") is destroyed. During which, this is called:

void win_iocp_socket_service_base::destroy( 
  win_iocp_socket_service_base::base_implementation_type& impl)
  // Remove implementation from linked list of all implementations.
  boost::asio::detail::mutex::scoped_lock lock(mutex_);

Obviously, you can guess what mutex_ is holding, right? A lovely pointer to an ex-pesky ex-critical (and probably ex-)section. So when scoped_lock's ctor does this: mutex_.lock(), this gets called:

void lock()

Windows looks at it and says "Why, how quaint! Didn't you just tell me, a few nano-seconds ago, to get rid of this? You're probably having memory problems. But I've got just the thing to solve that. Here, have a SegFault! Oh, and a nice day, too, of course". Or, as the duck said "And brother, when it disintegrates, it disintegrates".

So, now what? Well, I've decided to apply to socket the same treatment I gave io_service, namely, it has gone from tcp::socket sock to unique_ptr<tcp::socket> sock.

Next time, the final part of this moving adventure - adding move semantics to the channel classes. And deciding whether I want to allow move when a command is executing.

That won't be the end of it, naturally. I'll have to make this thread-safe, and then I'll have to revisit all this. But, one step at a time.

Tuesday, 5 February 2013

Introducing move semantics in my project

And then, there'll be many

A bit of a long post today, actually.

When I first started this pet project, in 2012-05, my goal was to (re)learn C++ and create a tool to automate tasks performed on several machines. Since that meant working via ssh, I turned to libssh2. When I later found boost asio, I thought it would be a good idea to use it as a framework for my work with libssh2.

At that time, I googled «libssh2 boost asio», but didn't get much. I only found one developer sharing his work with libssh2 + asio, but his design was different from what I wanted to do. So, I went ahead, step by learning step, until I finally built my first working example... which, I know today, is chock full of errors.

During this time, I've had one comment from someone who also thought this would be a good idea. And today, on libssh2's bug tracker, someone mentioned using libssh2 and boost asio together.

I'm glad to see there's more people doing this. For now, if you search google for «libssh2 boost asio», my posts will be the top results. I hope someday I'll have some company.

Lessons in movement

When I first dabbled in C++, I never really came upon move semantics. Back then, I avoided copy ctors like the plague, and passed "expensive" objects by reference. And although Scott Meyer's books didn't have the item "Prefer references to pointers", it was a mantra I held, and it was something I used when classes needed handles to something they didn't own:

class SomeHandle

class SomeSuch
    SomeHandle& handle;

Fast forward to C++11 - move semantics are everywhere, which puts a wrench on my previous mantra, because a) I have to leave the "moved-from" object empty; and b) I have to reseat the "moved-to" object members on move-assignment. So, I realize I can no longer use references on classes where I want move semantics.

Do I want move semantics on my SSH classes? I can't foresee a scenario where I'd need it. Still, I figured, since this is a new subject to me, and these are a set of inter-related classes, I'll definitely learn something from trying to implement move semantics here.

And learn I did, indeed.

I already mentioned Lesson #1, above, regarding the use of references as data members (courtesy of the great folk at Stack Overflow). Lesson #2 came after I tested my move ctors, which promptly resulted in an app crash.

Moving targets

My central class is SessionConnection. Here's part of its definition:

std::unique_ptr<boost::asio::io_service> ios;
boost::asio::ip::tcp::socket sock;
SessionHandle sessionHandle;
RemoteSessionHandle remoteSessionHandle;
SessionAuthenticator sessionAuthenticator;

This order must be maintained, because sessionAuthenticator requires SSH handshake complete, which is performed by remoteSessionHandle; which requires an SSH session, which is created by sessionHandle; which requires a TCP connection, which is guaranteed by sock; which requires an io_service.

Now, if we look at class RemoteSessionHandle, we'll find this:

SocketPtr sock;
SessionHandlePtr sessionHandle;

Notice the "Ptr" suffix - both are pointers. Both are injected on construction. Since there's no ownership involved, both are naked pointers, no smarts needed here (oh, and, both used to be references).

Now, let's look at SessionConnection's current move ctor:

SessionConnection::SessionConnection(SessionConnection&& other) :
    reportStatus(move(other.reportStatus)), host(move(,  
    port(move(other.port)), user(move(other.user)), pwd(move(other.pwd)), 
    ios(move(other.ios)), sock(move(other.sock)),

How's that for simplicity? Here's the test program (SSHSession is a wrapper around the SSH functionality, and owns a SessionConnection):

int main(int argc, char *argv[])
        cout << "start" << endl;
        ConnectionInfo ci; = "";
        ci.port = "22";
        ci.user = "user";
        ci.pwd = "password";

        SSHSession sess(ci, boost::bind(ShowStatus, _1));
        SSHSession sess2(move(sess));

        sess2.ExecuteCommand("ls -l", boost::bind(ShowStatus, _1), true);

        cout << "end" << endl;
    catch (SSHBaseException& ex)
        cout << diagnostic_information(ex) << endl;

As I said above, this crashes (so much for simplicity, then). It crashes when sess2's dtor runs. Actually, it crashes here, in RemoteSessionHandle::DoCloseSession():

int rc = libssh2_session_disconnect(sessionHandle->GetSession(), 
    "Disconnect SSH session");

As to why... when we do this: SSHSession sess2(move(sess)), what happens to both - the old and the new - SessionConnection::remoteSessionHandles?

We're taking sess's existing SessionConnection (let's call it oldSC) and moving it to sess2's brand new SessionConnection (let's call it newSC), which means SessionConnection's move ctor gets called:

SessionConnection::SessionConnection(SessionConnection&& other) :
    ..., remoteSessionHandle(move(other.remoteSessionHandle)),...

You'll recall remoteSessionHandle.sessionHandle is a pointer, which is set up at remoteSessionHandle construction.

Before the move, we have this:
  • oldSC.remoteSessionHandle.sessionHandle points to oldSC.sessionHandle
  • newSC.remoteSessionHandle.sessionHandle doesn't exist yet.
After the move, we get this:
  • oldSC.remoteSessionHandle.sessionHandle is nullptr.
  • newSC.remoteSessionHandle.sessionHandle points to what oldSC.remoteSessionHandle.sessionHandle pointed to, i.e., oldSC.sessionHandle
However, oldSC (as well as sess) is now a "moved-from" object, and the C++ standard requires that it's destructible and assignable, and nothing more. In fact, we have these lines in main():

SSHSession sess2(move(sess));
sess2.ExecuteCommand("ls -l", boost::bind(ShowStatus, _1), true);

And the program's output is something like this:

SSHSession dtor
total 44
<ls -l result omitted>

So, I'd say gcc notices sess is moved from and not used again, and destroys it right there. And, as far as I can see, it's perfectly allowed to do so.

So, this means that, when sess2's/newSC's dtors are called, we have a whole bunch of pointers happily pointing at the memory space formerly known as sess.oldSC. Which is why sessionHandle->GetSession() crashes, because what sessionHandle points at doesn't exist anymore.

How to correct this? Like I said, these classes are very inter-related, so I'll introduce some friend relations here, along with a private interface for reseating these pointers. And, while I'm at it, I'll also make private the current public members that expose libssh2 elements.

More on this, after I test it.

Friday, 1 February 2013

And.. I'm back!

A long time since the last post. Not only was December a very busy month (as is usually the case with Decembers), but I've also had time for a small vacation, during which I decided to just goof off.

Still, I've since got back to work. What's been happening?
  1. My pet project and its required libs all build on Visual Studio 2012 (Desktop). I've also added my first conditional compilation code, because Visual C++ doesn't have deleted/defaulted methods.
  2. Reviewed existing comments, and added more Doxygen comments. Also added diagrams, after installing graphviz.
  3. Better use of typedefs, especially after a hard-to-notice bug where I used SomeType instead of SomeType& (which also showed me I still have a lot to learn about using gdb on Windows).
  4. Started creating move ctors for my classes. Which led me to replace reference data members with pointer data members, following the advice I got here. Normal use was not broken with these changes. However, move semantics are not yet implemented, and I'll have to investigate/redesign further. More about this soon.
  5. The previous point led me to yet another little problem with boost asio's io_service (did I already mention this class needs some management love?), namely, it's not move constructible. Which, in turn, led me to another redesign, using unique_ptr<io_service>.
Point 3 posed me a bit of a problem, and I'm still on the fence about how I solved it - where to declare these typedefs. I first considered placing the declarations where the actual types are defined - thus, SessionHandleRef would be declared in session_handle.h. However, I couldn't do that for someone else's types - e.g., boost classes. And also, that meant no forward declarations, since I would have to #include the header to use a typedef (I believe this could be solved for my own classes, by creating a header file with the forward declarations and the typedefs).

So, I've switched to declaring typedefs where they're used. As long as I don't redefine the typedef (i.e., don't change its type-declaration), I'll be fine. On the one hand, it fits well with C++'s philosophy of "declare just before use"; on the other hand, it fells a bit disorganized to have the same typedef repeated in several headers (and this is why I'm not so sure about my solution). Still, it's nothing that can't be solved with a bit of search-and-replace, if the need arises.

I'm also setting up a way to make the code available. More on that after I get the move ctors sorted out.