My 2 cents inlined. On Wed, Dec 17, 2014 at 9:20 AM, Dominic Hamon <[email protected]> wrote: > > 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. >
+1. Thinking about object lifetimes instead of direct calls to new/delete is a next level of abstraction, that allows a programmer to concentrate more on what to do and not how to do. > > > > > > > > > > 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. > Exactly. Each way of passing objects around serves as an implicit documentation of the programmer's intent. In the Master, for example, I think most of things like Framework* will remain unaffected. > > > > > > > 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.* >
