On Tue, Dec 16, 2014 at 4:16 PM, Benjamin Hindman <[email protected]> wrote: > > > > > The style guide has the following to say (long quote, sorry): > > > > To be clear to other people on the dev list, this is from Google's style > guide, which we use in Mesos except for explicit places where we diverge. I > say this because std::unique_ptr might be one such place, and we shouldn't > eliminate that as an option if it makes sense. > > Now I'm a big fan of explicit ownership and moving ownership rather than > > sharing non-smart pointers, but I recognise that in our code-base, > > ownership is difficult to reason about in many cases. However, we do have > > quite a few cases where we manage lifetime scope with explicit delete > calls > > and I'd like to start by eliminating those. Ie, using std::unique_ptr to > > manage lifetime not necessarily ownership. > > > > I'm also a big fan explicit ownership and move semantics, more generally of > introducing abstractions that can help us write more robust and reliable > code. But I do want to point out that memory leaks, the problem it sounds > like you're addressing with using std::unique_ptr to manage lifetime, has > been a very minor source of bugs in our code base for the last 5 years. > > Moreover, in places where we've used raw pointers (especially in > libprocess), the safety that has come from forcing a programmer to think > about what's happening when they write 'delete pointer;' has been hugely > valuable in eliminating bugs that could now become more common. >
It sounds like you're suggesting that we should eschew modern C++ features that encourage safe code because forcing developers to use unsafe abstractions encourages them to think more and thus be safer in general. If that's the case, I couldn't disagree more. We should embrace the safer abstractions to ease the development of safer code, and lower the barrier of entry for new contributors coming to what is a complex code-base. > > > > This is difficult though as we may pass these pointers to other methods > or > > even other libprocess processes. In cases that we can reason about the > > lifetime of the various pointers, that should be fine, but we have to be > > careful. > > > > This is where I'd love to see some examples of how we'd use > std::unique_ptr. In the Master, for example, we pass things like Framework* > and Task* around, how would we do the same with std::unique_ptr? What would > be the new convention? These are the questions I'd love to see asked and > examples shown that everyone can discuss before making a decision we might > later regret. > I think the examples already being discussed in this thread have started to cover this. Developers need to understand the lifetimes of the objects they're passing around (as they do now) to make a call between std::unique_ptr + raw pointer/reference passing and std::shared_ptr. > > One option is to start by replacing these pointers with process::Owned. The > > downside to this approach is that it introduces more non-standard types > > and, because Owned is implemented using std::shared_ptr, doesn't move us > > closer to defining clear ownership. > > > > I don't think we should be afraid of introducing non-standard types like > Owned. If a non-standard type makes the code more readable, or can provide > value that the standard doesn't provide, it's a no brainer. This has > happened countless times in Mesos, and we've almost always decided to go > for something non-standard to make code easier to maintain going forward. > Of course, because historically libprocess has been streets ahead of the standard in terms of the abstractions you developed. Now that the language is finally catching up with the abstractions you created, we should consider moving to them. The main benefit is to lower the barrier of entry for new contributors. On a deeper level, I'd like to see committers from Mesos involved in C++ standards development to continue to push the language forward. Imagine if stout/libprocess could form the foundation of C++17s async and monadic abstractions. > > I do understand the value of being able to expose a public API that doesn't > require our non-standard types, but I don't think that's a hugely pressing > issue right now. > > Also, it would be great to see Owned reimplemented in terms of > std::unique_ptr along with any necessary cleanups. > > So the floor is open. Do we: > > > > To answer this question I'd love to see more examples from within our > codebase that can drive how we introduce std::unique_ptr. > > Ben. > -- Dominic Hamon | @mrdo | Twitter *There are no bad ideas; only good ideas that go horribly wrong.*
