6

A few days ago my friend told me about the situation, they had in their project. Someone decided, that it would be good to destroy the object of NotVerySafeClass in parallel thread (like asynchronously). It was implemented some time ago. Now they get crashes, because some method is called in main thread, while object is destroyed. Some workaround was created to handle the situation.

Ofcourse, this is just an example of not very good solution, but still the question:

Is there some way to prevent the situation internally in NotVerySafeClass (deny running the methods, if destructor was called already, and force the destructor to wait, until any running method is over (let's assume there is only one method))?

Mat
  • 202,337
  • 40
  • 393
  • 406
Werolik
  • 923
  • 1
  • 9
  • 23
  • Why not destroy the object after the method is called instead of in another tread? This is a design flaw, and needs to be fixed in the design, not via a patch. – RvdK Sep 13 '11 at 09:08
  • We can start the method after the destructor has started and when destroying isn't finished yet. The question is: can we prevent this? – Werolik Sep 13 '11 at 09:16

8 Answers8

8

No, no and no. This is a fundamental design issue, and it shows a common misconception in thinking about multithreaded situations and race conditions in general.

There is one thing that can happen equally likely, and this is really showing that you need an ownership concept: The function calling thread could call the function just right after the object has been destroyed, so there is no object anymore and try to call a function on it is UB, and since the object does not exist anymore, it also has no chance to prevent any interaction between the dtor and a member function.

PlasmaHH
  • 15,673
  • 5
  • 44
  • 57
  • Query from err 'challenged' C++ developer:) Could the NVSC destructor be overridden so as to take some action that does not result in the immediate deallocation of the object? Maybe the NVSC instance could be put on a pool queue to be re-used later, or perhaps queued to a thread that deallocates it after some wait time? – Martin James Sep 13 '11 at 09:49
  • @MartinJames: The dtor does not do the deallocation, this is done by the operator delete which in theory could be overloaded for this class. But this does not solve anything, as by deleting the instance will call the dtor, which destroys the object. You can not do anything against that. Accessing it thereafter is UB (think of changed vptrs, things that happen automatically for every dtor). and things get worse of course if your object was not even newed. – PlasmaHH Sep 13 '11 at 10:07
  • @MartinJames: What you could do is to make sure any thread only deletes it after it does not need it anymore. As others have suggested, using smart pointers (possibly a boost::shared_ptr) is a good idea here. – PlasmaHH Sep 13 '11 at 10:08
  • Thanks, @PlasmaHH. Rest assured that I won't be trying this anytime soon :) – Martin James Sep 13 '11 at 10:13
  • This kind behavior is reproducable even without mutithreading. Think of VC++, using Windows Messages. If you have a long waiting loop (waiting for socket or something), then you would have message pumping inside wait loop. If now a message comes, that causes destruction of 'this' object (ie. dialog is closed), than you have invalid object the moment you get out of the DispatchMessage call. – Bojan Hrnkas Mar 26 '21 at 14:25
2

What you need is a sound ownership policy. Why is the code destroying the object when it is still needed?

Without more details about the code, a std::shared_ptr would probably solve this issue. Depending on your specific situation, you may be able to solve it with a more lightweight policy.

R. Martinho Fernandes
  • 228,013
  • 71
  • 433
  • 510
1

Sounds like a horrible design. Can't you use smart pointer to make sure the object is destroyed only when no-one holds any references to it?

If not, I'd use some external synchronization mechanism. Synchronizing the destructor with a method is really awkward.

Eran
  • 21,632
  • 6
  • 56
  • 89
1

There is no methods that can be used to prevent this scenario.

In multithread programming, you need to make sure that an object will not be deleted if there are some others thread still accessing it.

If you are dealing with such code, it needs fundamental fix

metapora
  • 41
  • 2
1

(Not to promote bad design) but to answer your two questions:

... deny running the methods, if destructor was called already

You can do this with the solution proposed by @snemarch and @Simon (a lock). To handle the situation where one thread is inside the destructor, while another one is waiting for the lock at the beginning of your method, you need to keep track of the state of the object in a thread-safe way in memory shared between threads. E.g. a static atomic int that is set to 0 by the destructor before releasing the lock. The method checks for the int once it acquires the lock and bails if its 0.

... force the destructor to wait, until any running method is over

The solution proposed by @snemarch and @Simon (a lock) will handle this.

Anton Belov
  • 534
  • 4
  • 10
0

No. Just need to design the program propertly so that it is thread safe.

Ed Heal
  • 59,252
  • 17
  • 87
  • 127
0

Why not make use of a mutex / semaphore ? At the beginning of any method the mutex is locked, and the destructor wait until the mutex is unlocked. It's a fix, not a solution. Maybe you should change the design of a part of your application.

Simon
  • 860
  • 7
  • 23
0

Simple answer: no.

Somewhat longer answer: you could guard each and every member function and the destructor in your class with a mutex... welcome to deadlock opportunities and performance nightmares.

Gather a mob and beat some design sense into the 'someone' who thought parallel destruction was a good idea :)

snemarch
  • 4,958
  • 26
  • 38