[
https://issues.apache.org/jira/browse/GEODE-4599?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Blake Bender updated GEODE-4599:
--------------------------------
Description:
In several places NC tries to acquire a lock, but never checks that the lock
was successfully acquired. ACE documentation clearly states that this is bad
behavior:
{noformat}
Code like this is dangerous:
{
ACE_Guard<ACE_Lock> g(lock);
... perform critical operation requiring lock to be held...
}
Instead, one must do something like this:
{
ACE_Guard<ACE_Lock> g(lock);
if (! g.locked())
{
... handle error ...
}
else
{
... perform critical operation requiring lock to be held ...
}
}
{noformat}
It's worth noting that this may be linked to GEM-267 where the following code
was found with an error thrown inside a critical section of
{{ThinClientRegion.cpp}}.
{noformat}
if (m_resultCollectorLock != NULL) {
ACE_Guard < ACE_Recursive_Thread_Mutex > guard(
*m_resultCollectorLock);
m_rc->addResult(result);
} else {
m_rc->addResult(result);
}
{noformat}
We may need to investigate on a case-by-case basis how we should handle when
the lock isn't successfully acquired. There are roughly 200 other places where
this same behavior is present.
was:
In several places NC tries to acquire a lock, but never checks that the lock
was successfully acquired. ACE documentation clearly states that this is bad
behavior:
{noformat}
Code like this is dangerous:
{
ACE_Guard<ACE_Lock> g(lock);
... perform critical operation requiring lock to be held...
}
Instead, one must do something like this:
{
ACE_Guard<ACE_Lock> g(lock);
if (! g.locked())
{
... handle error ...
}
else
{
... perform critical operation requiring lock to be held ...
}
}
{noformat}
It's worth noting that this may be linked to GEM-267 where the following code
was found with an error thrown inside a critical section of
{{ThinClientRegion.cpp}}.
{noformat}
if (m_resultCollectorLock != NULL) {
ACE_Guard < ACE_Recursive_Thread_Mutex > guard(
*m_resultCollectorLock);
m_rc->addResult(result);
} else {
m_rc->addResult(result);
}
{noformat}
We may need to investigate on a case-by-case basis how we should handle when
the lock isn't successfully acquired. There are roughly 200 other places where
this same behavior is present.
> Ensure native client checks if locks are acquired
> -------------------------------------------------
>
> Key: GEODE-4599
> URL: https://issues.apache.org/jira/browse/GEODE-4599
> Project: Geode
> Issue Type: Wish
> Components: native client
> Reporter: Addison
> Priority: Major
>
> In several places NC tries to acquire a lock, but never checks that the lock
> was successfully acquired. ACE documentation clearly states that this is bad
> behavior:
> {noformat}
> Code like this is dangerous:
> {
> ACE_Guard<ACE_Lock> g(lock);
> ... perform critical operation requiring lock to be held...
> }
> Instead, one must do something like this:
> {
> ACE_Guard<ACE_Lock> g(lock);
> if (! g.locked())
> {
> ... handle error ...
> }
> else
> {
> ... perform critical operation requiring lock to be held ...
> }
> }
> {noformat}
> It's worth noting that this may be linked to GEM-267 where the following code
> was found with an error thrown inside a critical section of
> {{ThinClientRegion.cpp}}.
> {noformat}
> if (m_resultCollectorLock != NULL) {
> ACE_Guard < ACE_Recursive_Thread_Mutex > guard(
> *m_resultCollectorLock);
> m_rc->addResult(result);
> } else {
> m_rc->addResult(result);
> }
> {noformat}
> We may need to investigate on a case-by-case basis how we should handle when
> the lock isn't successfully acquired. There are roughly 200 other places
> where this same behavior is present.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
