[ 
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)

Reply via email to