Project

General

Profile

Bug #3601

Calling event.put( std::move( aUniquePtr ) ) leaves aUniquePtr in a state that looks usable, but is deadly

Added by Adam Lyon over 7 years ago. Updated about 7 years ago.

Status:
Closed
Priority:
Normal
Category:
I/O
Target version:
Start date:
03/15/2013
Due date:
% Done:

100%

Estimated time:
Occurs In:
Scope:
Internal
Experiment:
-
SSI Package:
Duration:

Description

Calling

event.put( std::move( aUniquePtr ) );

should leave aUniquePtr in a state where it can only be deleted or assigned (e.g. set to nullptr). Instead, aUniquePtr is left still pointing to the original object, which looks usable but is in fact invalid. We think this is leading to seemingly random segfaults in optimized code (and leads to intermittent segfaults in debug code when the service holding on to the unique_ptr is deleted).

History

#1 Updated by Christopher Green over 7 years ago

  • Category set to I/O
  • Status changed from New to Feedback
  • Assignee set to Christopher Green

Hi,

The more I think about this the more convinced I am that there is no actual bug in art. Moreover, the workaround we suggested to you on Friday (use of std::unique_ptr::release() actually introduces a memory leak rather than preventing a memory corruption.

Allow me to explain:

When you pass something to a function that requires an r-value reference, it must either be unnamed or converted to an r-value reference by use of std::move(). You must expect that the object passed in this way be assignable or destructible only after the call (since std::unique_ptr is guaranteed to have a zero pointer after a move copy, we have a little more latitude here, so @reset() is allowable, for example). However, this is not guaranteed to be the case.

This is what actually happens in art 1.03.08: when you call art::Event::put(std::unique_ptr<PROD> &&prod), you are passing a reference, which is then further passed (as a reference) to art::Event::put(std::unique_ptr<PROD> &&prod, std::string instance_name) without initiating a move (or other) copy. It is then passed (also as a reference) to art::Wrapper<PROD>::Wrapper(std::unique_ptr<PROD> && prod), at which point the object pointed to by the unique_ptr (not the pointer value, note) is move-copied into the art::Wrapper's by-value data member obj of type PROD. This last operation explains why you were seeing an empty vector in your std::unique_ptr previously: this is what is expected with what art actually does with the r-value reference it receives, although it is not safe to rely on this behavior.

At this point, the only concrete instance of std::unique_ptr in existence is the one you created. That means you still have the ownership, and that std::unique_ptr must be reset() (not release()) in order to ensure correct behavior.

I am willing to believe that a memory corruption problem exists, but I am now less inclined to believe it exists in art. I believe we should be looking elsewhere for the segfault in your system. I would be happy to get together with you to analyze your execution with valgrind and/or a debugger to help isolate your memory corruption problem. Please let me know when you would be available.

#2 Updated by Christopher Green about 7 years ago

Did this investigation resolve, Adam?

#3 Updated by Christopher Green about 7 years ago

  • Description updated (diff)

#4 Updated by Christopher Green about 7 years ago

  • Target version set to 1.07.00

Commit 7c72824 addresses the confusion caused when the unique_ptr remained valid after a put() call. The unique_ptr is now empty and the pointed-to object has been finalized following the call to put().

#5 Updated by Christopher Green about 7 years ago

  • Description updated (diff)

#6 Updated by Christopher Green about 7 years ago

  • Status changed from Feedback to Resolved
  • % Done changed from 0 to 100

Resolved due to lack of feedback

#7 Updated by Christopher Green about 7 years ago

  • Status changed from Resolved to Closed


Also available in: Atom PDF