"delete this"

[Michael_Tanner]

I've been snooping around trying to figure whether or not this was completely safe. I've noticed that if you were to simply "delete this" you might have occurrences where the deleted pointer is still in use elsewhere in the program. To prevent this, you'd need to dereference it and perform a check to ensure that the pointer is valid.

Now, the question: is using const_cast on the this pointer safe?

I've whipped up a quick example below:

#include "stdafx.h"

class myclass
{
private:
	int *integer;

public:

	myclass(int a, int b)
	{
		integer = new int[1];

		*integer = a - b;

		if(!lessthannaught(*integer))
		{
			delete this;

			const_cast<myclass *>(this) = 0;
		}
	}

	~myclass()
	{
		delete integer;
		printf("Deconstructor\n");
	}

	bool lessthannaught(int i)
	{
		return i < 0;
	}
};

int main()
{
	myclass *_class;

	_class = new myclass(3, 1);

	if(!_class)
		printf("Memory freed from within constructor\n");
	else
	{
		printf("Freeing\n");
		delete _class;
	}

	printf("----------\n");

	_class = new myclass(2, 3);

	if(!_class)
		printf("Memory freed from within constructor\n");
	else
	{
		printf("Freeing\n");
		delete _class;
	}

	while(1);

	return 0;
}

I've tested it and it appears to work, but is such a method recommended (or better yet, are there alternate methods around it)? Thanks!

[mike_2000_17]

Wow, "delete this;"... I can't begin to tell you how unsafe this is.

>>is such a method recommended?
Here is my scale in C++:
- Desirable (includes things like RAII, exception-safety, binary compatibility, generic implementations, concept checking, const-correctness, etc.)
- Recommended (includes things like programming-by-contract, independence, abstraction, encapsulation, non-copyable, smart pointers, etc.)
- Acceptable (includes things like the visitor pattern, dynamic casts, etc.)
- Lesser evil (includes things like singleton patterns, fly-weight idiom, union-types, raw-pointers, etc.)
- Last resort (includes very few things that are horrible, like const-casts and C-style casts, for example)
- Forbidden, horrible, disgusting, evil code (includes almost nothing, but it includes "delete this;")

What if your object is allocated statically (not with new)?
What if your object is allocated as part of an array?
What if there exists another pointer to your object?
...
These are all perfectly normal uses of a class in C++. And they would all lead to a disaster if you decide to do a "delete this;" statement. The const_cast and setting this to null is completely useless. You have contrived an example in which this works, but that's just a very special case. In most cases, the this pointer won't relate to anything meaningful in the caller's code (in a method, the this pointer should always be treated as a read-only local variable, there is no guarantee that treating it any differently will be valid).

If you want to achieve a similar effect in a safe manner, there are a few options. I would suggest using a smart pointer and a factory function. The smart pointer will make sure that your class has sole ownership of the object. And the factory function will make sure that there is only one way to create your object.

class myclass {
  private:
    int mData; //hold some data.. whatever
    boost::scoped_ptr<myclass> mThis; //holds a smart "this" pointer, enforces sole ownership.

    myclass() : data(42), mThis(this) { }; //the private constructor forces the outside code not to be able to create an object of myclass.
    myclass(const myclass&); //disables copying the object.
    myclass& operator=(const myclass&); //disables assignment-copy of the object.

  public:

    //public factory function:
    static const boost::scoped_ptr<myclass>& Create() {
      return (new myclass())->mThis; //return the smart pointer.
    };
    //public destructor function:
    void Destroy() {
      boost::scoped_ptr<myclass> temp;
      temp.swap(mThis);
    };
};

Yet, the above will be safe, as far as I can tell, but it also makes the use of a pointer to your object a reference, which means that it can't be stored in an STL container and it will make copying it around a bit harder.

All in all, I would recommend that you explain what your real problem is, because I am sure a better solution can be found (better than the one above, and certain better than "delete this;"!).

[Michael_Tanner]

I was intending on using it in just that manner. I just wanted to optimize some code (which by you is somewhat the opposite), hoping to have a class where the constructor would call internal methods and granted their success its instance would continue to exist. I.e. I could've scrapped the constructor code and dealt with it in the main function (granted "integer" is a public variable):

if(!_class->lessthannaught(_class->*integer))
{
    delete _class;
    _class = 0;
}

[mike_2000_17]

This is exactly what exceptions are used for. If you throw an exception in the constructor (whenever some condition is not fulfilled or something failed), it will cause a stack unwinding which will effectively unconstruct your object (not call its destructor, but literally back-track all the code in the constructor). The exception is then reported to the user-side code which will then be responsible to dealing with the situation. That's the only way and the best way to deal with a constructor that could fail.

[arkoenig]

To give you an idea of how many worms are in this particular can, I'd like to isolate two lines from the OP's code:

delete this;
const_cast<myclass *>(this) = 0;

The program is already broken at this point, because once you have executed "delete this;" any subsequent use of this is invalid.

Come to think of it, the second statement is invalid anyway, even without "delete this", because this is not an lvalue. And even if it were, casting it to myclass* yields an rvalue, to which you can't assign.

If your compiler accepts this program, your compiler is broken.

[Michael_Tanner]

Ah well, thanks guys. I suppose the shortest route isn't always the best one.

[mike_2000_17]

@arkoenig:
>>And even if it were, casting it to myclass* yields an rvalue, to which you can't assign.

rvalue-lvalue qualification is not the same as cv-qualification. You can have a const or non-const, lvalue or rvalue variable. The standard specifies that the this pointer should be "non-lvalue", which is really weird IMO, because "non-lvalue" is a ISO C term, not a C++ term, it is not defined anywhere in the standard. One interpretation of non-lvalue is simply that it is a const lvalue in C++, the other interpretation is that it is an rvalue. If the OP's compiler chooses to take the first interpretation, then the const_cast of the this pointer would be acceptable under conversion rules that apply to const_cast. In that case, the this pointer is a const lvalued local variable (or a const lvalued reference), so the const_cast strips the const, but this is already just a local variable, and thus, setting it to NULL is just meaningless because that won't have any effect outside the member function.

But you are right, if the OP's compiler accepts the code he posted, it surely means it is not a good compiler, but I'm not sure it actually breaks the standard. I can easily imagine why the compiler would choose to do this. Imagine, in a const member function, you attempt to do a const_cast on a data member to be able to modify it (of course, that's ugly, but allowed). Then, if the this pointer is an const rvalue pointer, then that would make the data member also a const rvalue and the const_cast would fail. To avoid this, when designing a compiler, you could either use a special rule for dereferencing the this pointer which does not carry the rvalued-ness, or you could just make the this pointer a const lvalue. The practical difference between the two is of very little significance since no-one would ever dream of treating the this pointer as anything but an rvalue. However, the latter option is the easiest approach to implement, probably what the OP's compiler chose.

>>because once you have executed "delete this;" any subsequent use of this is invalid.
That's not entirely true. After "delete this", any subsequent _dereferencing_ of the this pointer is invalid (i.e. accessing any of the data members or virtual member functions is invalid because it dereferences the this pointer). But, in theory, just reading or writing the value of the this pointer (not what it points to) is "valid" (if the compiler permits it), just like this code is valid, but useless:

void someFunction(int* some_ptr) {
  delete some_ptr;
  std::cout << "The pointer I just deleted had address: " << some_ptr << std::endl;
  some_ptr = NULL; // reading or writing some_ptr is valid and well-behaved after the delete. This also applies to the this pointer.
};

[arkoenig]

rvalue-lvalue qualification is not the same as cv-qualification. You can have a const or non-const, lvalue or rvalue variable.

I think you're missing the point.

The C++ standard, clause 5.2.11, says that the result of const_cast<T>(e) is an lvalue if type T is a reference type; otherwise the result is an rvalue. So const_cast<myclass*>(anything) is an rvalue, because the type myclass* is not a reference type.

But, in theory, just reading or writing the value of the this pointer (not what it points to) is "valid" (if the compiler permits it), just like this code is valid, but useless:

void someFunction(int* some_ptr) {
  delete some_ptr;
  std::cout << "The pointer I just deleted had address: " << some_ptr << std::endl;
  some_ptr = NULL; // reading or writing some_ptr is valid and well-behaved after the delete. This also applies to the this pointer.
};

I still believe you are mistaken. One of the interesting aspects of pointers is that it is undefined behavior even to copy the value of a pointer to memory that has been freed. Clause 3.7.3.2 of the C++ standard says that once memory has been deallocated, any pointer to that memory becomes invalid, and any use of such a pointer, even to pass it to a function, results in undefined behavior.

This behavior is inherited from C, and comes from some processor architectures that use different registers for addresses and other data. In such processors, the mere act of putting an address into an address register causes the processor to do address translation, which in turn entails verifying that the pointer points to memory that is actually allocated.

The requirement that every pointer actually point to allocated memory (with the sole exception of the null pointer) implies that the following program fragment has undefined behavior:

int* p = new int;
delete p;
int* q = p;   // undefined

The reason is that after the delete statement, p no longer points to allocated memory. C++ permits the compiler to generate code for the initialization of q that puts the value of p into an address register; because that value does not point to memory that is currently allocated, the processor is under no obligation to execute this initialization.

This example is exactly analogous to the "delete this;" example I cited earlier. Because "this" does not point to allocated memory after the deletion, any attempt to copy it results in undefined behavior.

[mike_2000_17]

@Michael_Tanner:

I'm curious. What compiler did you use which allows the original code you posted to compile? Please provide the version number too.

I've been googling a little bit on the subject and I am now pretty much convinced that a standard-compliant compiler should not allow this to compile. Maybe it is a bug in the compiler or a very esoteric one.

[Michael_Tanner]

"C++ Optimizing Compiler Version 15.00.21022.08 for 80x86"