1

I have some intermittent segmentation faults in a Qt application. I think the problem is related to our (bad) use of QSharedPointer. The Qt Documentation states :

QSharedPointer::QSharedPointer ( T * ptr ) : Creates a QSharedPointer that points to ptr. The pointer ptr becomes managed by this QSharedPointer and must not be passed to another QSharedPointer object or deleted outside this object.

I think we are doing both must not... :/

Is there a OOP way to enforce that the pointer managed by QSharedPointer cannot be deleted or passed to another QSharedPointer?

The best solution will be to have a compiler error.

demonplus
  • 5,613
  • 12
  • 49
  • 68
Symbiosoft
  • 4,681
  • 6
  • 32
  • 46

4 Answers4

8

The normal pattern is to put the new statement inside the smart pointer's constructor, like this:

QSharedPointer<Obj> p (new Obj(2));

That way you never have a reference to the naked pointer itself.

If you refactor your code so that all new operator are in lines like these, all your problems will be solved.

drxzcl
  • 2,952
  • 1
  • 26
  • 28
  • 2
    NOTE: All your problems of these kind will be solved. Statement does not apply to unrelated problems ;) – drxzcl Nov 11 '10 at 14:16
  • Well, you can still use `data()` (`get()` for `std::shared_ptr`) and shoot yourself in the foot. – Martin Ba Nov 11 '10 at 14:31
  • That's more or less what I said, but I don't think that even this will solve the problem. In fact, it will create new problems with tracking down the lifespan of an object. I cannot recommend this method for the problem at hand. – San Jacinto Nov 11 '10 at 14:59
  • You are correct, if you are not clear about object ownership/lifetime you will be in a world of hurt regardless. – drxzcl Nov 11 '10 at 15:18
1

Well, an OOP-esque way would be to create the raw pointer as a private member in a wrapper class, and only perform actions on the pointer through methods that act on the shared pointer. kind of silly though, isn't it?

Or you could make your class with the raw pointer a base class to your other classes and make the raw pointer a private member in the class. In this regard, you're more or less creating an abstract class that does nothing. Your derivative classes must instead do all the work, and since they can't access the raw pointer, compilation will fail... this doesn't stop someone from just copying the raw pointer value out of the shared pointer, though.

In the end, I think your best policy is to manuall change all of the functions in question to use either a shared pointer or else a raw pointer. You can copy one shared pointer to another safely, so why no just go that way?

Edit: I might add that regardless of whether or not you're using shared pointers, it sounds like you're having ownership issues. If a pointer was created in one scope, it should be deleted in that scope, unless the function that it is passed to contractually takes ownership of the pointer. Using a shared pointer in this scenario will only caused different bugs, eventually. It sounds like you have design issues deeper than just the sharing of pointers.

San Jacinto
  • 8,774
  • 5
  • 43
  • 58
  • "It sounds like you have design issues deeper than just the sharing of pointers." Indeed we have. I was not in charge of that chunk of code when our "architect" decided (without informing anyone and telling us why) to change some pointers to QSharedPointer at some place but not at other places in the code. Now I am in charge of that code and it is like a big spaghetti bowl... ;) – Symbiosoft Nov 11 '10 at 14:31
1

I'm not familiar with the particular Qt implementation of a shared pointer, but as a general guideline : attempting to mix raw pointers with managed pointers usually ends in blood. Once you 'trust' a shared pointer implementation in taking ownership of your dynamically allocated data, you should under no circumstances try to manage the object lifetime yourself (for example by deleting the provided pointer).

Is there a OOP way to enforce that the pointer managed by QSharedPointer cannot be deleted ?

I guess you could imagine some weird technique where the pointed type would have a private destructor and declare QSharedPointer as friend (which would effectively prevent any 'outside deletion' from compiling), but I wouldn't bet that anything good can come out of this (and note that it will make your type absolutely unusable unless new'ed and transfered to a QSharedPointer).

Is there a OOP way to enforce that the pointer managed by QSharedPointer cannot be passed to another QSharedPointer?

I can't think of any, and that is another reason why you should avoid manipulating the raw pointer once it's ownership has been transferred to a QSharedPointer.

icecrime
  • 74,451
  • 13
  • 99
  • 111
0

Check your code for all .data() usage and make sure what they return is neither stored nor deleted. I don't think a hard compiler error would be good, because sometimes it's okay to pass the raw pointer, e.g. to a function that doesn't store nor delete passed pointers. (Especially when using 3rd-party code, you cannot always change everything to use shared pointers, and often you want it to work with both, raw and shared ptrs). One could mark QSharedPointer::data() as deprecated (by patching Qt), to get a compile time warning.

Frank Osterfeld
  • 24,815
  • 5
  • 58
  • 70