[ https://issues.apache.org/jira/browse/TS-3156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14206549#comment-14206549 ]
Alan M. Carroll commented on TS-3156: ------------------------------------- Sorry, I misread your question. I think it's just an accident that it takes ProxyMutex* because the smart pointers are intrusive. For this reason a ProxyMutex* is just as good as a Ptr<ProxyMutex> and is a little faster (since you don't have any temporary smart pointers). Because the reference count is in ProxyMutex wrapping a pointer in a smart pointer isn't taking ownership and so this API isn't taking ownership either. I will note that passing the raw pointer can be faster than passing a smart pointer because you don't have to construct/destruct temporaries (which involve atomic operations on the reference count). Your scenario ProxyMutex foo; { MutexLock(lock, &foo, this_ethread()); <-- takes lock and smart pointer owners it with ref count 1 } <-- lock object is destroyed and since smart pointer has ref count of zero when dying it will free ProxyMutex; will break but I suspect the original implementors decided they'd rather have the faster locks and simply not do that. I'm not sure I've encountered any code where a raw ProxyMutex is declared in that way. An alternative solution to this is to make ProxyMutex::ProxyMutex() private and require the use of new_ProxyMutex() to actually get one. You can see this is the intent from the comment in the ProxyMutex class declaration, which says "do not use this constructor". On Monday, November 10, 2014 7:24 PM, Powell Molleti (JIRA) <j...@apache.org> wrote: [ https://issues.apache.org/jira/browse/TS-3156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14205749#comment-14205749 ] Powell Molleti commented on TS-3156: ------------------------------------ Hi Alan, It does take ownership of the object since the api says: class MutexLock { private: Ptr<ProxyMutex> m; public: MutexLock(ProxyMutex * am, EThread * t):m(am) }; It is perfectly ok from compiler perspective to use it as follows: ProxyMutex foo; { MutexLock(lock, &foo, this_ethread()); <-- takes lock and smart pointer owners it with ref count 1 } <-- lock object is destroyed and since smart pointer has ref count of zero when dying it will free ProxyMutex; The constructor should be MutexLock(Ptr<ProxyMutex> m, EThread * t); and the call should be enforced as MutexLock(lock, Ptr<ProxyMutex>(&foo), this_ethread()); to force auto refcount up. Let me know. Powell -- This message was sent by Atlassian JIRA (v6.3.4#6332) > Mutex[Try]Lock bool() operator change and unused API removal > ------------------------------------------------------------ > > Key: TS-3156 > URL: https://issues.apache.org/jira/browse/TS-3156 > Project: Traffic Server > Issue Type: Improvement > Components: Core > Reporter: Powell Molleti > Assignee: James Peach > Priority: Minor > Labels: review > Fix For: 5.2.0 > > Attachments: MutexLock-ats.patch, MutexLock-ats.patch, > Use-Ryo-s-patch-to-pass-shared_ptr-to-MutexLock.patch, fix-MutexLock.patch > > > Removed unused constructor in MutexLock along with set_and_take() method, had > to change FORCE_PLUGIN_MUTEX() for that. Removed release() method. > default bool and ! operator from both MutexLock and MutexTryLock with > is_locked() API. Changes if (lock) to if (lock.is_locked()) across the code > base. > Ran make test will be performing more system testing. Posted before for early > comments / feedback. -- This message was sent by Atlassian JIRA (v6.3.4#6332)