[
https://issues.apache.org/jira/browse/TS-3156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14206549#comment-14206549
]
Alan M. Carroll edited comment on TS-3156 at 11/11/14 7:14 PM:
---
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
PtrProxyMutex 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
{code}
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;
{code}
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.
was (Author: amc):
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
PtrProxyMutex 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-tabpanelfocusedCommentId=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:
PtrProxyMutex 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(PtrProxyMutex m, EThread * t);
and the call should be enforced as MutexLock(lock, PtrProxyMutex(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
