fuCtor / QAMQP

AMQP implementation for Qt
Other
28 stars 3 forks source link

New message recevied but messageReceived signal was not emitted #43

Closed MaplerStory closed 10 years ago

MaplerStory commented 10 years ago

the condition before emit messagaReceived signal has some problems : if(message->leftSize == 0 && messages_.size() == 1) { emit pq_func()->messageReceived(pq_func()); }

if a new message has been enqueued and the signal was emitted, but the message has not been dequeue from the queue, then new messages will not emit the signal because messages_.size() is not equal to 1, it's > 1 .(bind the messagaReceived signal to a new thread and dequeue new message in the new thread may cause this problem. send a message to the queue with empty body cause this problem 100%)

i don't understand why use the " messages_.size() == 1" condition. does it designed for some purpose?

mbroadst commented 10 years ago

@MaplerStory please try the updated version of this library at https://github.com/mbroadst/qamqp, it's the new home of QAMQP

MaplerStory commented 10 years ago

@mbroadst thank you, i have noticed this version, but your version could not be built on Windows for now. i would like to use it on Windows, just may be like what fuCtor does.

btw, i notice that your queue's implement is just like this version, so your version may also has this problem: send a message with empty playload to the queue, no signal will be emitted. (but i have not tested this case.)

and i suggest that u may use smart pointer to store Message in the queue, just like fuCtor 's implement.

mbroadst commented 10 years ago

@MaplerStory ah yes, I unfortunately do not have a windows box to build on (I would gladly accept patches if you can get it working). I'll take a look into the test case, do you have code that exhibits this problem? Either way, this codebase is no longer being maintained I believe, so if you'd like to work together I'm sure we can solve your problem.

There is no need to use a smart pointer to store the messages in the queue, as they are now implicitly shared class (http://qt-project.org/doc/qt-5/implicit-sharing.html)

mbroadst commented 10 years ago

issue fixed with this commit: https://github.com/mbroadst/qamqp/commit/84746ff77c0216951edf9121ebc3b251dbceb7b5

mbroadst commented 10 years ago

@MaplerStory fixed the windows build with: https://github.com/mbroadst/qamqp/commit/7e151dfc7b483e03ed31e4987196c8a9c7f69573

MaplerStory commented 10 years ago

@mbroadst oh, i got the latest version and built it on windows. it built well. thank u! :+1:

mbroadst commented 10 years ago

@MaplerStory great to hear :) let us know over at https://github.com/mbroadst/qamqp if you have any other issues