wrong check for sent bytes in AMQPConnection

[marcelog]

diff --git a/amqp.inc b/amqp.inc
the actual code only checks for false as the return value of fwrite in AMQPConnection::write($data). sometimes, fwrite() does not return false but 0 bytes, and the while(true) makes php hang forever. this is the main concern here. the main loop and that fwrite is not returning false when an errno=32 happens. my solution (others may exist.. ):

index 1bf03b3..3607afe 100644
--- a/amqp.inc
+++ b/amqp.inc
@@ -436,7 +436,7 @@ class AMQPConnection extends AbstractChannel
$len = strlen($data);
while(true)
{

•        if(false === ($written = fwrite($this->sock, $data)))
  

•        if(false === ($written = fwrite($this->sock, $data)) || $written === 0)
       {
           throw new Exception ("Error sending data");
       }
  

thanks!

[videlalvaro]

Hi, I understand the issue… still I don't if this wouldn't break some other things? What do you think?

[marcelog]

hi!

i wouldn't know about other things.. i've just started using the library, but this particular patch is applied to my copy and seems to work ok :). mind you that i'm not doing anything "rare" nor is it a production environment, just testing rabbitmq through this library.

i think the while(true) loops should really be avoided and taken care of, since this break things in software that depends on this library, but should be the topic of another issue :)

anything you'd like me to try with this patch so you can commit it or change the implementation in some other way that fix this particular issue? just let me know

best!

[aib]

Hmm, I remember fixing this, but I guess not...

If I remember correctly, there are about 4 different ways to check for a socket close. (false, 0, feof and metadata["timed_out"], off the top of my head) I also remember your solution working for the majority of cases.

However, in theory a socket write operation could write 0 bytes on a connected socket - if the outgoing buffer was full. I don't know if that's the case with stream wrappers, but it's something to look out for.