Issue 3161: _out types and nested calls (cxx_revision) Source: Triodia Technologies Pty Ltd (Mr. Michi Henning, michi(at)triodia.com) Nature: Uncategorized Issue Severity: Summary: consider: // IDL struct VariableStruct { ... }; interface I { void foo(out VariableStruct s); void bar(out VariableStruct s); }; Then: // C++ void MyImplForI::foo(VariableStruct_out s) { bar(s); bar(s); // Leak here } void MyImplForI::bar(VariableStruct_out s) { s = new VariableStruct; } The freeing of memory for out params relies on the default conversion by the _out constructor from a pointer to the _out type which, as a side effect, frees the memory return by the previous call. However, in this case, and _out param is passed to another call, so the assignment operator runs, not the constructor: T_out& operator=(T* p) { ptr_ = p; return *this; } The assignment operator doesn't free the previous memory, so we get the leak. Should the assignment operator be changed? Resolution: Revised Text: Actions taken: December 22, 1999: received issue Discussion: deferred in June 2011 to the next RTF End of Annotations:===== ]Date: Wed, 22 Dec 1999 12:47:03 +1000 (EST) From: Michi Henning Reply-To: C++ Revision Task Force To: C++ Revision Task Force cc: issues@omg.org Subject: _out types and nested calls Message-ID: Organization: Object Oriented Concepts MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-UIDL: njIe9W_od932 From: Steve Vinoski Subject: Re: _out types and nested calls In-Reply-To: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" X-UIDL: RW\d9YB]d9C6[d9/nPe9 At 12:47 PM 12/22/99 +1000, Michi Henning wrote: >consider: > > // IDL > > struct VariableStruct { ... }; > > interface I > { > void foo(out VariableStruct s); > void bar(out VariableStruct s); > }; > >Then: > > > // C++ > void > MyImplForI::foo(VariableStruct_out s) > { > bar(s); > bar(s); // Leak here > } > > void > MyImplForI::bar(VariableStruct_out s) > { > s = new VariableStruct; > } > >The freeing of memory for out params relies on the default conversion >by the _out constructor from a pointer to the _out type which, as a >side effect, frees the memory return by the previous call. No, this is wrong. The automatic freeing of memory for an out param can only occur when a _var is passed as the out param. The _out type behaves just like a pointer except that it has a constructor from a _var that causes any memory the _var owns to be freed. If you pass a raw pointer as an out, you are responsible for managing it. Similarly, if you pass an _out as a parameter, it behaves (for a var-length type) as a pointer, and you again have to explicitly manage it. This is by design. >However, >in this case, and _out param is passed to another call, so the >assignment operator runs, not the constructor: > > T_out& operator=(T* p) { ptr_ = p; return *this; } > >The assignment operator doesn't free the previous memory, so we get >the leak. The _out parameter is a by-value VariableStruct_out, so you're assigning into a copy. The _out is designed to behave like a raw pointer in the servant method body, so the assignment operator shouldn't have anything to do with it. If the parameter type were a raw pointer, you'd get the same exact behavior. >Should the assignment operator be changed? I don't think any changes are needed here. --steve Date: Wed, 22 Dec 1999 16:20:48 +1000 (EST) From: Michi Henning To: Steve Vinoski cc: C++ Revision Task Force Subject: Re: _out types and nested calls In-Reply-To: <4.1.19991221225043.00c54a90@mail.boston.amer.iona.com> Message-ID: Organization: Object Oriented Concepts MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-UIDL: _[)e9=dJ!!>`'e9Wa2!! On Tue, 21 Dec 1999, Steve Vinoski wrote: > >The freeing of memory for out params relies on the default conversion > >by the _out constructor from a pointer to the _out type which, as a > >side effect, frees the memory return by the previous call. > > No, this is wrong. The automatic freeing of memory for an out param can > only occur when a _var is passed as the out param. The _out type behaves > just like a pointer except that it has a constructor from a _var that > causes any memory the _var owns to be freed. If you pass a raw pointer as > an out, you are responsible for managing it. Similarly, if you pass an _out > as a parameter, it behaves (for a var-length type) as a pointer, and you > again have to explicitly manage it. This is by design. Sure, I understand what's going on. Here is my concern: basically, the _out params are meant to be transparent. If I pass a _var where an _out is expected, the automatic conversion takes care of deleting the memory. However, if I'm in the body of an operation want to pass an out param to another operation, I have to be careful with the leak I described. The question is whether that couldn't be fixed by changing the assignment operator. As far as I can see, _out types are otherwise never assigned to each other (but I may be overlooking something). This is mainly a usability and "least surprise" issue -- I guess you are saying that if I use an _out param in this way, it behaves like a raw pointer, not like a _var. From a usability point of view, it would be nicer if it behaved like a _var... Cheers, Michi. -- Michi Henning +61 7 3891 5744 Object Oriented Concepts +61 4 1118 2700 (mobile) Suite 4, 904 Stanley St +61 7 3891 5009 (fax) East Brisbane 4169 michi@ooc.com.au AUSTRALIA http://www.ooc.com.au/staff/michi-henning.html Reply-To: From: "Nick Sharman" To: "Michi Henning" , "Steve Vinoski" Cc: "C++ Revision Task Force" Subject: RE: _out types and nested calls Date: Wed, 22 Dec 1999 09:43:39 -0000 Message-ID: <001201bf4c61$06a30970$5610a8c0@thumper.uk.peerlogic.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Priority: 3 (Normal) X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook 8.5, Build 4.71.2173.0 Importance: Normal X-MimeOLE: Produced By Microsoft MimeOLE V4.72.3110.3 In-Reply-To: Content-Type: text/plain; charset="iso-8859-1" X-UIDL: )@(e9,gEe9LPQd9S*>!! Michi Henning wrote: > > On Tue, 21 Dec 1999, Steve Vinoski wrote: > > > >The freeing of memory for out params relies on the default conversion > > >by the _out constructor from a pointer to the _out type which, as a > > >side effect, frees the memory return by the previous call. > > > > No, this is wrong. The automatic freeing of memory for an out param can > > only occur when a _var is passed as the out param. The _out type behaves > > just like a pointer except that it has a constructor from a _var that > > causes any memory the _var owns to be freed. If you pass a raw > pointer as > > an out, you are responsible for managing it. Similarly, if you > pass an _out > > as a parameter, it behaves (for a var-length type) as a pointer, and you > > again have to explicitly manage it. This is by design. > > Sure, I understand what's going on. Here is my concern: basically, the > _out params are meant to be transparent. If I pass a _var where an _out > is expected, the automatic conversion takes care of deleting the memory. > This freeing happens before control enters the operation body; by the time control gets inside the operation, the target of the _out just contains meaningless bits, just as if an raw, uninitialized _out were passed directly. > However, if I'm in the body of an operation want to pass an out param > to another operation, I have to be careful with the leak I described. > Once inside the operation, the _out is just an _out; it doesn't have any memory of the fact that it was created from a _var. A fortiori, the coder of the operation doesn't know whether or when _vars get passed, so they have to assume responsibility for managing the _out's memory. > The question is whether that couldn't be fixed by changing the assignment > operator. As far as I can see, _out types are otherwise never assigned > to each other (but I may be overlooking something). > I don't think this is possible, without also requiring that a raw _out parameter must be initialized to null or a freeable address before the call. Otherwise, the assignment would attempt to free illegal or still-useful addresses. We end up in a situation where it's impossible to tell whether we need to free something (to avoid a _var leak) or not (to avoid a crash). Which is why _out types got added in the first place... > This is mainly a usability and "least surprise" issue -- I guess you are > saying that if I use an _out param in this way, it behaves like a raw > pointer, not like a _var. From a usability point of view, it would be > nicer if it behaved like a _var... > I think you can arrive at a consistent story with the current rules: *provided an operation is well-behaved*, then using a _var at the call site means the call site manages memory without leaks. If the callee leaks (whether or not the surplus point got stored in the _out parameter), then that's its responsibility. Regards Nick Date: Thu, 23 Dec 1999 10:49:00 +1000 (EST) From: Michi Henning To: C++ Revision Task Force Subject: Re: _out types and nested calls Message-ID: Organization: Object Oriented Concepts MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-UIDL: @()!!MI*!!d94!!~+%!! > > Sure, I understand what's going on. Here is my concern: basically, the > > _out params are meant to be transparent. If I pass a _var where an _out > > is expected, the automatic conversion takes care of deleting the memory. > > > This freeing happens before control enters the operation body; by the time > control gets inside the operation, the target of the _out just contains > meaningless bits, just as if an raw, uninitialized _out were passed > directly. Nick, that's not right. The constructor for an _out param frees the memory and then sets the pointer to null: T_out(T_var& p) : ptr_(p.ptr_) { delete ptr_; ptr_ = 0; } Consider the following code (which is more likely to occur than the one I showed previously): void X_impl::foo(Y_out y) { bar_ref->bar(y); if (some_condition(y)) { another_bar_ref->bar(y); // Leak here } } Now, if the copy constructor for _out were to free memory, that leak wouldn't occur. Similarly, the assignment operator from _out to _out could do the same thing. As far as I can see, that change would hurt no-one, but would help to prevent the above mistake. Cheers, Michi. -- Michi Henning +61 7 3891 5744 Object Oriented Concepts +61 4 1118 2700 (mobile) Suite 4, 904 Stanley St +61 7 3891 5009 (fax) East Brisbane 4169 michi@ooc.com.au AUSTRALIA http://www.ooc.com.au/staff/michi-henning.html X-Sender: vinoski@mail.boston.amer.iona.com X-Mailer: QUALCOMM Windows Eudora Pro Version 4.1 Date: Wed, 22 Dec 1999 22:45:15 -0500 To: Michi Henning From: Steve Vinoski Subject: Re: _out types and nested calls Cc: C++ Revision Task Force In-Reply-To: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" X-UIDL: FZF!!LOL!!o)6e9\ALe9 At 10:49 AM 12/23/99 +1000, Michi Henning wrote: >Consider the following code (which is more likely to occur than the >one I showed previously): > > void > X_impl::foo(Y_out y) > { > bar_ref->bar(y); > > if (some_condition(y)) { > another_bar_ref->bar(y); // Leak here > } > } > >Now, if the copy constructor for _out were to free memory, that leak >wouldn't occur. Similarly, the assignment operator from _out to _out >could do the same thing. > >As far as I can see, that change would hurt no-one, but would help >to prevent the above mistake. The change would break all the code that already correctly handles cases like this. Why penalize those who already have correctly coded their methods (by making them go back and change code that already works) just for those who don't Purify? --steve Date: Thu, 23 Dec 1999 13:47:28 +1000 (EST) From: Michi Henning To: Steve Vinoski cc: C++ Revision Task Force Subject: Re: _out types and nested calls In-Reply-To: <4.1.19991222224308.00b64600@mail.boston.amer.iona.com> Message-ID: Organization: Object Oriented Concepts MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-UIDL: !k!"!-T%!!LWg!!l The change would break all the code that already correctly handles cases > like this. Why penalize those who already have correctly coded their > methods (by making them go back and change code that already works) just > for those who don't Purify? Yes, strong argument. Changing the semantics of something that's already there isn't such a hot idea, as a rule. OK, I had my try and I'll give up now ;-) I'm convinced that changing this would do more harm than good. Cheers, Michi. -- Michi Henning +61 7 3891 5744 Object Oriented Concepts +61 4 1118 2700 (mobile) Suite 4, 904 Stanley St +61 7 3891 5009 (fax) East Brisbane 4169 michi@ooc.com.au AUSTRALIA http://www.ooc.com.au/staff/michi-henning.html From: Paul Kyzivat To: "'Michi Henning'" , C++ Revision Task Force Subject: RE: _out types and nested calls Date: Mon, 3 Jan 2000 10:51:24 -0500 MIME-Version: 1.0 X-Mailer: Internet Mail Service (5.5.2448.0) Content-Type: text/plain; charset="iso-8859-1" X-UIDL: 8>+e9iOSd9LQ>e9RFR!! I may have lost the context of this discussion over the holidays, so forgive me if I say something that doesn't make sense in context... Originally, _out params were documented as being transparent except for their behavior with _var types. But now that they are present, they do provide added options not otherwise possible. Specifically, as long as they *always* initialize the caller's arg pointer to null (for T*, T_var, and T_out arguments), then one never need worry about seeing an uninitialized value. So, in other contexts, a non-null value in an _out can be assumed to be suitable for freeing. The example Michi gives is a good one. Another one that could be caught is: void X_impl::foo(Y_out y1, Y_out y2) { bar_ref->bar(y1); y2 = y1; // will result in extra frees // should be forced to use _retn or _duplicate } void X_impl::zoo(Y_out y1) Y_var my_y; bar_ref->bar(my_y); y1 = my_y._retn(); // ... bar_ref->bar(my_y); y1 = my_y._retn(); // leak } It seems to me that assignment between _outs is just as dangerous as assignment between _vars, and should be a compile error, because it is ambiguous where ownership is intended to reside. And assignment into an out should free any prior value. Paul > -----Original Message----- > From: Michi Henning [mailto:michi@ooc.com.au] > Sent: Wednesday, December 22, 1999 7:49 PM > To: C++ Revision Task Force > Subject: Re: _out types and nested calls > > > > > > Sure, I understand what's going on. Here is my concern: > basically, the > > > _out params are meant to be transparent. If I pass a _var > where an _out > > > is expected, the automatic conversion takes care of > deleting the memory. > > > > > This freeing happens before control enters the operation > body; by the time > > control gets inside the operation, the target of the _out > just contains > > meaningless bits, just as if an raw, uninitialized _out were > passed > > directly. > > Nick, that's not right. The constructor for an _out param > frees the memory > and then sets the pointer to null: > > T_out(T_var& p) : ptr_(p.ptr_) { delete ptr_; ptr_ = 0; } > > Consider the following code (which is more likely to occur than the > one I showed previously): > > void > X_impl::foo(Y_out y) > { > bar_ref->bar(y); > > if (some_condition(y)) { > another_bar_ref->bar(y); // Leak here > } > } > > Now, if the copy constructor for _out were to free memory, that leak > wouldn't occur. Similarly, the assignment operator from _out to _out > could do the same thing. > > As far as I can see, that change would hurt no-one, but would help > to prevent the above mistake. > > Cheers, > > Michi. > -- > Michi Henning +61 7 3891 5744 > Object Oriented Concepts +61 4 1118 2700 (mobile) > Suite 4, 904 Stanley St +61 7 3891 5009 (fax) > East Brisbane 4169 michi@ooc.com.au > AUSTRALIA > http://www.ooc.com.au/staff/michi-henning.html > Date: Mon, 03 Jan 2000 12:37:01 -0500 From: Marc Laukien Organization: Object Oriented Concepts, Inc. X-Mailer: Mozilla 4.7 [en] (WinNT; I) X-Accept-Language: en MIME-Version: 1.0 To: Paul Kyzivat CC: "'Steve Vinoski'" , Michi Henning > , C++ Revision Task Force Subject: Re: _out types and nested calls References: <9B164B713EE9D211B6DC0090273CEEA9140287@bos1.noblenet.com> Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=us-ascii X-UIDL: fj4!!-!B!!2fWd9G'#!! Hi Paul, > void > X_impl::zoo(Y_out y) > { > Y_var my_y, another_y; > bar_ref->bar(my_y); > > if (some_condition(my_y)) { > another_bar_ref->bar(another_y); > my_y = another_y; // won't compile ^^^^^^^^^^^^^^^^ Why shouldn't this compile? This looks perfectly legal to me. From page 1-8 of the C++ mapping spec: // C++ B_ptr bp = ... A_ptr ap = bp; // implicit widening Object_ptr objp = bp; // implicit widening objp = ap; // implicit widening B_var bv = bp; // bv assumes ownership of bp ap = bv; // implicit widening, bv retains // ownership of bp obp = bv; // implicit widening, bv retains // ownership of bp A_var av = bv; // illegal, compile-time error A_var av = B::_duplicate(bv);// av, bv both refer to bp B_var bv2 = bv; // implicit _duplicate A_var av2; av2 = av; // implicit _duplicate ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > my_y = another_y._retn(); // legal way > } > } Cheers, Marc -- Marc Laukien Phone: (978) 439 9285 x 245 Object Oriented Concepts, Inc. FAX: (978) 439 9286 44 Manning Rd. WWW: http://www.ooc.com Billerica, MA 01821 E-Mail: mailto:ml@ooc.com From: Paul Kyzivat To: "'Marc Laukien'" , Paul Kyzivat Cc: "'Steve Vinoski'" , Michi Henning , C++ Revision Task Force Subject: RE: _out types and nested calls Date: Mon, 3 Jan 2000 13:32:09 -0500 MIME-Version: 1.0 X-Mailer: Internet Mail Service (5.5.2448.0) Content-Type: text/plain; charset="iso-8859-1" X-UIDL: >1hd9MaX!!S&O!!p From: Marc Laukien [mailto:ml@ooc.com] > Why shouldn't this compile? This looks perfectly legal to me. Oops. I guess I still haven't awakened from vacation yet. Kind of screws up my counter-example. But my basic point is that in some cases we are trying hard to force compiler errors where users could easily screw up and we have enough information to be helpful. But in this particular case we have the information to be helpful but aren't doing it. Sender: jbiggar@corvette.floorboard.com Message-ID: <3870EEF2.D4524D38@floorboard.com> Date: Mon, 03 Jan 2000 10:48:18 -0800 From: Jonathan Biggar X-Mailer: Mozilla 4.7 [en] (X11; U; SunOS 5.6 sun4u) X-Accept-Language: en MIME-Version: 1.0 To: Marc Laukien CC: Paul Kyzivat , "'Steve Vinoski'" , Michi Henning , C++ Revision Task Force Subject: Re: _out types and nested calls References: <9B164B713EE9D211B6DC0090273CEEA9140287@bos1.noblenet.com> <3870DE3D.667751C7@ooc.com> Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=us-ascii X-UIDL: [?N!!=2;!!M'C!!afBe9 Marc Laukien wrote: > > Hi Paul, > > > void > > X_impl::zoo(Y_out y) > > { > > Y_var my_y, another_y; > > bar_ref->bar(my_y); > > > > if (some_condition(my_y)) { > > another_bar_ref->bar(another_y); > > my_y = another_y; // won't compile > ^^^^^^^^^^^^^^^^ > > Why shouldn't this compile? This looks perfectly legal to me. From > page > 1-8 of the C++ mapping spec: Marc is correct that this should compile. It didn't use to, under older versions of the C++ mapping. -- Jon Biggar Floorboard Software jon@floorboard.com jon@biggar.org Date: Tue, 4 Jan 2000 09:58:01 +1000 (EST) From: Michi Henning To: Paul Kyzivat cc: "'Marc Laukien'" , "'Steve Vinoski'" , C++ Revision Task Force Subject: RE: _out types and nested calls In-Reply-To: <9B164B713EE9D211B6DC0090273CEEA914028D@bos1.noblenet.com> Message-ID: Organization: Object Oriented Concepts MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-UIDL: 0%Qd9Mbf!!S-&e91+9e9 On Mon, 3 Jan 2000, Paul Kyzivat wrote: > Kind of screws up my counter-example. But my basic point is that in some > cases we are trying hard to force compiler errors where users could easily > screw up and we have enough information to be helpful. But in this > particular case we have the information to be helpful but aren't doing it. I agree with your point. On the other hand, Steve's argument is a very strong one: if we change the semantics now, we will likely break existing code. Given that what we have now works (even though it may not be quite optimal), breaking code isn't really an option. Cheers, Michi. -- Michi Henning +61 7 3891 5744 Object Oriented Concepts +61 4 1118 2700 (mobile) Suite 4, 904 Stanley St +61 7 3891 5009 (fax) East Brisbane 4169 michi@ooc.com.au AUSTRALIA http://www.ooc.com.au/staff/michi-henning.html Date: Wed, 5 Jan 2000 11:08:26 +1000 (EST) From: Michi Henning To: C++ Revision Task Force Subject: Amendment to issue 3161 Message-ID: Organization: Object Oriented Concepts MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-UIDL: YdHe9)d