bugfix for #79411 Connect with invalid password exits #30
Conversation
The problem was that write was called on closed sockets. That caused SIGPIPE signals that weren't handled. It is fixed by introducing write_ignore_pipe_signal() that sets up a short-lived signal handler SIGPIPE that simply ignores the signal while write() is called. write_ignore_pipe_signal() is now called everywhere that write() used to be called directly.
|
It'd be even nicer to upgrade to newer rabbitmq-c which doesn't SIGPIPE on write, rather than deviate further from the library. Something like like #14 or going further and fully separating Net::RabbitMQ from rabbitmq-c would be great. |
|
I wasn't aware that Net::RabbitMQ included code from rabbitmq-c. :-( I looked at #14 "Upgrading to rabbitmq-c-fb6fca832fd2 and removing unsupported features.". Unfortunately, I tried checking out https://github.com/MattiL/Net--RabbitMQ.git (which includes #14) and the bug is still there in that version: process still exits with SIGPIPE under similar circumstances. Furthermore, that commit does several things: Upgrades and removes unsupported features. In addition, So it looks like #14 needs to be redone or at least investigated, and rabbitmq-c commit 815f521 included. Currently trying to merge #14 into master causes massive merge conflicts, so that isn't straight-forward. But I agree that merging in a new rabbitmq-c is the solution. At this time, upgrading rabbitmq-c to newest version looks like a task that requires more insight into RabbitMQ and tools than I have, and is a little larger than I'm up to. In the meantime the SIGPIPE problem is still there. My comparatively tiny merge request fixes it. |
|
After a little more investigation, it looks like MattiL did the upgrade of rabbitmq-c (to some unknown version of rabbitmq-c) in #14. In that pull-request postwait commented:
It looks like another unknown but older version of rabbitmq-c was imported in the very first commit in this project in 2009, and hasn't been updated since. But there have been approx 850 lines of diff since then in the originally imported rabbitmq-c files: I just took a look of some of these diffs, and they look non-trivial to port over. rabbitmq-c has changed a lot since 2009. A question for omniti-labs, postwait & core contributors: Is there a way we could cleanly use rabbitmq-c as-is? And turn any necessary changes we need in rabbitmq-c into a clean pull-request for that project? That way it would also be much easier in the future to keep up with master/HEAD in rabbitmq-c in the future. Is there a plan for that? Alternatively, is there a plan for how to keep up with the many, many changes in rabbitmq-c? It seems to me that rabbitmq-c was effectively forked in 2009 for this project, and the two projects have diverged significantly since. Of course this is high-priority for me :-) - but I can't imagine a worse category of bug: This pull-request fixes the problem where if one restarts the rabbitmq server, then on the next operation any perl client process exits without an error message or anything, and eval doesn't even do the trick. I hope the desire to eventually upgrade / keep up with rabbitmq-c does not prevent pulling this issue for now. |
|
We've done a bit of work locally to try and do this split without having to compromise too much on the feature loss. Unfortunately we haven't yet been able to get a basic return callback working (this requires modifications to rabbitmq-c to let us inspect the local memory queue of messages). You can see the work in progress at: https://github.com/emarcotte/Net--RabbitMQ. I just rolled up all of our current work and submitted it. |
|
Hi emarcotte, I tried to visit https://github.com/emarcotte/Net--RabbitMQ, but it dead (404). Do you have the work in progress somewhere else? Peter |
|
@pmorch Sorry it should be back shortly, sorry still in the process of pushing it to github |
This is a fix for Bug #79411 for Net--RabbitMQ: Connect with invalid password exits
The problem was that write was called on closed sockets. That caused
SIGPIPE signals that weren't handled.
It is fixed by introducing write_ignore_pipe_signal() that sets up a
short-lived signal handler SIGPIPE that simply ignores the signal
while write() is called.
write_ignore_pipe_signal() is now called everywhere that write() used to
be called directly.