{"id":334,"date":"2014-09-08T15:15:20","date_gmt":"2014-09-08T19:15:20","guid":{"rendered":"http:\/\/blog.mozilla.org\/nfroyd\/?p=334"},"modified":"2014-09-08T15:15:20","modified_gmt":"2014-09-08T19:15:20","slug":"xpcom-and-move-constructors","status":"publish","type":"post","link":"https:\/\/blog.mozilla.org\/nfroyd\/2014\/09\/08\/xpcom-and-move-constructors\/","title":{"rendered":"xpcom and move constructors"},"content":{"rendered":"

Benjamin Smedberg recently announced that he was handing over XPCOM module ownership duties<\/a> to me.\u00a0 XPCOM contains basic data structures used throughout the Mozilla codebase, so changes to its code can have wide-ranging effects.\u00a0 I’m honored to have been given responsibility for a core piece of the Gecko platform.<\/p>\n

One issue that’s come up recently and I’m sure will continue to come up is changing XPCOM data structures to support two new C++11 features, rvalue references and their killer app, move constructors.\u00a0 If you aren’t familiar with C++11’s new rvalue references feature, I highly recommend C++ Rvalue References Explained<\/a>.\u00a0 Move constructors are already being put to good use elsewhere in the codebase, notably mozilla::UniquePtr<\/a><\/tt>, which can be used to replace XPCOM’s nsAutoPtr<\/tt> and nsAutoRef<\/tt> (bug 1055035<\/a>).\u00a0 And some XPCOM data structures have received the move constructor treatment, notably nsRefPtr<\/tt> (bug 980753<\/a>) and nsTArray<\/tt> (bug 982212<\/a>).<\/p>\n

A recent discussion<\/a> and the associated bug<\/a>, however, decided that the core referenced-counted smart pointer class in XPCOM, nsCOMPtr<\/tt>, shouldn’t support move constructors.\u00a0 While move constructors could have replaced the already_AddRefed<\/tt> usage associated with nsCOMPtr<\/tt>, such as:<\/p>\n

already_AddRefed<nsIMyInterface>\r\nNS_DoSomething(...)\r\n{\r\n  nsCOMPtr<nsIMyInterface> interface = ...;\r\n  \/\/ do some initialization stuff\r\n  return interface.forget();\r\n}<\/pre>\n
with the slightly shorter:<\/p>\n
nsCOMPtr<nsIMyInterface>\r\nNS_DoSomething(...)\r\n{\r\n  nsCOMPtr<nsIMyInterface> interface = ...;\r\n  \/\/ do some initialization stuff\r\n  return interface;\r\n}<\/pre>\n
There were two primary arguments against move constructor support.\u00a0 The first argument was that the explicitness of having to call .forget()<\/tt> on an nsCOMPtr<\/tt> (along with the explicitness of the already_AddRefed<\/tt> type), rather than returning it, is valuable for the code author, the patch reviewer, and subsequent readers of the code.\u00a0 When dealing with ownership issues in C++, it pays to be more explicit, rather than less.\u00a0 The second argument was that due to the implicit conversion of nsCOMPtr<T><\/tt> to a bare T*<\/tt> pointer (a common pattern in smart pointer classes), returning nsCOMPtr<T><\/tt> from functions makes it potentially easy to write buggy code:<\/p>\n
\/\/ What really happens in the below piece of code is something like:\r\n\/\/\r\n\/\/ nsIMyInterface* p;\r\n\/\/ {\r\n\/\/   nsCOMPtr<nsIMyInterface> tmp(NS_DoSomething(...));\r\n\/\/   p = tmp.get();\r\n\/\/ }\r\n\/\/\r\n\/\/ which is bad if NS_DoSomething is returning the only ref to the object.\r\n\/\/ p now points to deleted memory, which is a security risk.\r\nnsIMyInterface* p = NS_DoSomething(...);<\/pre>\n
(I should note that we can return nsCOMPtr<T><\/tt> from functions today, and in most cases, thanks to compiler optimizations<\/a>, it will be as efficient as returning already_AddRefed<\/tt>.\u00a0 But Gecko culture is such that a function returning nsCOMPtr<T><\/tt> would be quite unusual, and therefore unlikely to pass code review.)<\/p>\n
The changes to add move constructors to nsRefPtr<\/tt> and nsTArray<\/tt>?\u00a0 They were reviewed by me.\u00a0 And the nixing of move constructors for nsCOMPtr<\/tt>?\u00a0 That was also done by me (with a lot of feedback from other people).<\/p>\n
I accept the charge of inconsistency.\u00a0 However, I offer the following defense.\u00a0 In the case of nsTArray<\/tt>, there are no ownership issues like there are with nsCOMPtr<\/tt>: you either own the array, or you don’t, so many of the issues raised about nsCOMPtr<\/tt> don’t apply in that case.<\/p>\n
For the case of nsRefPtr<\/tt>, it is true that I didn’t seek out as much input from other people before approving the patch.\u00a0 But the nsRefPtr<\/tt> patch was also done without the explicit goal of removing already_AddRefed<\/tt> from the code base, which made it both smaller in scope and more palatable.\u00a0 Also, my hunch is that nsRefPtr<\/tt> is used somewhat less than nsCOMPtr<\/tt> (although this may be changing somewhat given other improvements in the codebase, like WebIDL), and so it provides an interesting testbed for whether move constructors and\/or less explicit transfers of ownership are as much of a problem as argued above.<\/p>\n","protected":false},"excerpt":{"rendered":"
Benjamin Smedberg recently announced that he was handing over XPCOM module ownership duties to me.\u00a0 XPCOM contains basic data structures used throughout the Mozilla codebase, so changes to its code can have wide-ranging effects.\u00a0 I’m honored to have been given responsibility for a core piece of the Gecko platform. One issue that’s come up recently […]<\/p>\n","protected":false},"author":320,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[1],"tags":[540,72421],"_links":{"self":[{"href":"https:\/\/blog.mozilla.org\/nfroyd\/wp-json\/wp\/v2\/posts\/334"}],"collection":[{"href":"https:\/\/blog.mozilla.org\/nfroyd\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/blog.mozilla.org\/nfroyd\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/blog.mozilla.org\/nfroyd\/wp-json\/wp\/v2\/users\/320"}],"replies":[{"embeddable":true,"href":"https:\/\/blog.mozilla.org\/nfroyd\/wp-json\/wp\/v2\/comments?post=334"}],"version-history":[{"count":0,"href":"https:\/\/blog.mozilla.org\/nfroyd\/wp-json\/wp\/v2\/posts\/334\/revisions"}],"wp:attachment":[{"href":"https:\/\/blog.mozilla.org\/nfroyd\/wp-json\/wp\/v2\/media?parent=334"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/blog.mozilla.org\/nfroyd\/wp-json\/wp\/v2\/categories?post=334"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/blog.mozilla.org\/nfroyd\/wp-json\/wp\/v2\/tags?post=334"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}