You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by mg...@apache.org on 2011/03/30 21:11:09 UTC

svn commit: r1087047 - /qpid/trunk/qpid/cpp/src/qpid/sys/cyrus/CyrusSecurityLayer.cpp

Author: mgoulish
Date: Wed Mar 30 19:11:09 2011
New Revision: 1087047

URL: http://svn.apache.org/viewvc?rev=1087047&view=rev
Log:
qpid-3171

The registration of the codec happens on a different thread from the
use of the codec.  It is possible for the registration to occur after
the first attempted use.  In my testing, this happened 3% of the time
-- 165 times out of 5000 tests -- when using RDMA transport, and 0 times
out of 5000 when using TCP.  Which is why we didn't notice it earlier.

We have a function that tells when we are ready to encode --
CyrusSecurityLayer::canEncode.  But it does not check the validity of
the codec pointer before using it, so it cores in this situation.

I believe simply checking that pointer is probably the best solution.
Introducing that check caused the crash not to show up in 10,000
trials.  There were also no hangs.


Modified:
    qpid/trunk/qpid/cpp/src/qpid/sys/cyrus/CyrusSecurityLayer.cpp

Modified: qpid/trunk/qpid/cpp/src/qpid/sys/cyrus/CyrusSecurityLayer.cpp
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/sys/cyrus/CyrusSecurityLayer.cpp?rev=1087047&r1=1087046&r2=1087047&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/sys/cyrus/CyrusSecurityLayer.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/sys/cyrus/CyrusSecurityLayer.cpp Wed Mar 30 19:11:09 2011
@@ -106,7 +106,7 @@ size_t CyrusSecurityLayer::encode(const 
 
 bool CyrusSecurityLayer::canEncode()
 {
-    return encrypted || codec->canEncode();
+    return codec && (encrypted || codec->canEncode());
 }
 
 void CyrusSecurityLayer::init(qpid::sys::Codec* c)



---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:commits-subscribe@qpid.apache.org


Re: svn commit: r1087047 - /qpid/trunk/qpid/cpp/src/qpid/sys/cyrus/CyrusSecurityLayer.cpp

Posted by Gordon Sim <gs...@redhat.com>.
On 03/30/2011 08:11 PM, mgoulish@apache.org wrote:
> Author: mgoulish
> Date: Wed Mar 30 19:11:09 2011
> New Revision: 1087047
>
> URL: http://svn.apache.org/viewvc?rev=1087047&view=rev
> Log:
> qpid-3171
>
> The registration of the codec happens on a different thread from the
> use of the codec.  It is possible for the registration to occur after
> the first attempted use.  In my testing, this happened 3% of the time
> -- 165 times out of 5000 tests -- when using RDMA transport, and 0 times
> out of 5000 when using TCP.  Which is why we didn't notice it earlier.
>
> We have a function that tells when we are ready to encode --
> CyrusSecurityLayer::canEncode.  But it does not check the validity of
> the codec pointer before using it, so it cores in this situation.
>
> I believe simply checking that pointer is probably the best solution.
> Introducing that check caused the crash not to show up in 10,000
> trials.  There were also no hangs.

That might be a reasonable workaround, but if the issue is lack of 
coordination between two threads it sounds like there might be a more 
complete solution needed.

Can you elaborate a little on the threading issue?

>
>
> Modified:
>      qpid/trunk/qpid/cpp/src/qpid/sys/cyrus/CyrusSecurityLayer.cpp
>
> Modified: qpid/trunk/qpid/cpp/src/qpid/sys/cyrus/CyrusSecurityLayer.cpp
> URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/sys/cyrus/CyrusSecurityLayer.cpp?rev=1087047&r1=1087046&r2=1087047&view=diff
> ==============================================================================
> --- qpid/trunk/qpid/cpp/src/qpid/sys/cyrus/CyrusSecurityLayer.cpp (original)
> +++ qpid/trunk/qpid/cpp/src/qpid/sys/cyrus/CyrusSecurityLayer.cpp Wed Mar 30 19:11:09 2011
> @@ -106,7 +106,7 @@ size_t CyrusSecurityLayer::encode(const
>
>   bool CyrusSecurityLayer::canEncode()
>   {
> -    return encrypted || codec->canEncode();
> +    return codec&&  (encrypted || codec->canEncode());
>   }
>
>   void CyrusSecurityLayer::init(qpid::sys::Codec* c)
>
>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project:      http://qpid.apache.org
> Use/Interact: mailto:commits-subscribe@qpid.apache.org
>
>


---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org