qpid-proton C++ on_sendable race condition

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

qpid-proton C++ on_sendable race condition

Daniel Pocock


reSIProcate has a class QpidProtonThread[1] for sending to a topic.

The class runs the container in a thread, the method
QpidProtonThread::thread() is the code executed in the thread when it
starts.

Another thread is calling the sendMessage(..) method.  That method uses
event_loop()->inject(...) to have the doSend(...) method called safely.
This appears to work for a while.

I notice that when the event loop calls my void_function0 class
operator(), which calls doSend(...), it is not calling it in the
container thread, it is being called in the thread that invoked
sendMessage(...).  Is that expected behaviour?

Eventually, the on_sendable() method is called.  I see it is being
called in the container's thread.  That method also calls doSend(...).
At this point, the other thread becomes stuck.

I was thinking about simply removing the on_sendable() method.  However,
given that I am using the inject method, should I not be having a
problem like this at all?

Regards,

Daniel


1.
https://github.com/resiprocate/resiprocate/blob/master/repro/QpidProtonThread.cxx

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: qpid-proton C++ on_sendable race condition

astitcher
Sorry not to get back to you earlier - I've been at a conference for
the past few days.

Comments inline.

On Tue, 2017-06-27 at 12:00 +0200, Daniel Pocock wrote:
>
> reSIProcate has a class QpidProtonThread[1] for sending to a topic.
>
> The class runs the container in a thread, the method
> QpidProtonThread::thread() is the code executed in the thread when it
> starts.
>

I'm assuming this is an application method as I don't recognise that
name.

> Another thread is calling the sendMessage(..) method.  That method
> uses
> event_loop()->inject(...) to have the doSend(...) method called
> safely.
> This appears to work for a while.

Probably accidentally! The cross thread functionality was experimental
(and rather broken) in the current (0.17) release.
>
> I notice that when the event loop calls my void_function0 class
> operator(), which calls doSend(...), it is not calling it in the
> container thread, it is being called in the thread that invoked
> sendMessage(...).  Is that expected behaviour?

That is the expected, but broken, behaviour - I said that released code
was experimental!

>
> Eventually, the on_sendable() method is called.  I see it is being
> called in the container's thread.  That method also calls
> doSend(...).
> At this point, the other thread becomes stuck.

I expect it's a deadlock
.
>
> I was thinking about simply removing the on_sendable()
> method.  However,
> given that I am using the inject method, should I not be having a
> problem like this at all?

If you can wait a couple of weeks the new C++ IO code should get merged
to master and the 0.28 release made.

This should fix the problem you are having.

If you continue having problems then, or you want help trying the new
code on the branch. Let me know

Andrew


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: qpid-proton C++ on_sendable race condition

Daniel Pocock
On 29/06/17 19:28, Andrew Stitcher wrote:
> On Tue, 2017-06-27 at 12:00 +0200, Daniel Pocock wrote:
>> reSIProcate has a class QpidProtonThread[1] for sending to a topic.
>>
>> The class runs the container in a thread, the method
>> QpidProtonThread::thread() is the code executed in the thread when it
>> starts.
>>
> I'm assuming this is an application method as I don't recognise that
> name.

Correct, the method is in my code[1] on Github


>> Another thread is calling the sendMessage(..) method.  That method
>> uses
>> event_loop()->inject(...) to have the doSend(...) method called
>> safely.
>> This appears to work for a while.
> Probably accidentally! The cross thread functionality was experimental
> (and rather broken) in the current (0.17) release.
>> I notice that when the event loop calls my void_function0 class
>> operator(), which calls doSend(...), it is not calling it in the
>> container thread, it is being called in the thread that invoked
>> sendMessage(...).  Is that expected behaviour?
> That is the expected, but broken, behaviour - I said that released code
> was experimental!
>
>> Eventually, the on_sendable() method is called.  I see it is being
>> called in the container's thread.  That method also calls
>> doSend(...).
>> At this point, the other thread becomes stuck.
> I expect it's a deadlock
> .
>> I was thinking about simply removing the on_sendable()
>> method.  However,
>> given that I am using the inject method, should I not be having a
>> problem like this at all?
> If you can wait a couple of weeks the new C++ IO code should get merged
> to master and the 0.28 release made.
>
> This should fix the problem you are having.
>
> If you continue having problems then, or you want help trying the new
> code on the branch. Let me know

Do you mean 0.28 or 0.18?  Do you have any idea how soon it will be
released?

Ideally I would like to try the Debian package when it becomes available
but I don't mind building a local package manually from the branch to
test it if necessary.

Regards,

Daniel


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: qpid-proton C++ on_sendable race condition

astitcher
On Wed, 2017-08-02 at 16:06 +0100, Daniel Pocock wrote:

> ...
> > If you can wait a couple of weeks the new C++ IO code should get
> > merged
> > to master and the 0.28 release made.
> >
> > This should fix the problem you are having.
> >
> > If you continue having problems then, or you want help trying the
> > new
> > code on the branch. Let me know
>
> Do you mean 0.28 or 0.18?  Do you have any idea how soon it will be
> released?

That was a type - I mean 0.18. We're on the release path now, but there
are a number of bugs/issues still to go.

If you can try the current master branch, that will give us very useful
feedback.

>
> Ideally I would like to try the Debian package when it becomes
> available

From our perspective it would really help for you to test the code
before it's released, so that there is the best chance the released
code has no bugs that affect you.

> but I don't mind building a local package manually from the branch to
> test it if necessary.
>
> Regards,
>
> Daniel

Thanks

Andrew


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Loading...