Opened 8 years ago

Closed 8 years ago

#1180 closed defect (fixed)

msgq dies on socket send error

Reported by: shane Owned by: jelte
Priority: medium Milestone: Sprint-20111025
Component: ~msgq (obsolete) Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: Medium
Sub-Project: Core Feature Depending on Ticket:
Estimated Difficulty: 3 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

Jeremy reported an error to the bind10-dev list:

https://lists.isc.org/pipermail/bind10-dev/2011-August/002558.html

The included trace ends like this:

"/b/work/BIND10-systest/20110812084501-MacOS/build/src/bin/msgq/b10-msgq", 
line 344, in __send_data
    return sock.send(data)
socket.error: [Errno 32] Broken pipe

We need to wrap this, so that msgq does not exit.

Subtickets

Change History (10)

comment:1 Changed 8 years ago by shane

  • Component changed from Unclassified to msgq
  • Milestone set to Next-Sprint-Proposed
  • Sub-Project changed from DNS to Core

comment:2 Changed 8 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20111025

comment:3 Changed 8 years ago by jelte

  • Owner set to jelte
  • Status changed from new to assigned

comment:4 Changed 8 years ago by jelte

  • Estimated Difficulty changed from 0 to 3

comment:5 Changed 8 years ago by jelte

  • Owner changed from jelte to UnAssigned
  • Status changed from assigned to reviewing

Ready for review. As discussed on the planning call, I only caught the exception, and only SIGPIPE, and did not add any tests for this. If someone sees an easy way to do so, we can, but i'd prefer spending time looking at a replacement :)

Minor other notes; this module also doesn't use our logging framework, so as the error response (apart from dropping the send action and closing the socket), I just added a print(), as the rest of this code uses. Also, my editor found some whitespace it automatically removed.

comment:6 Changed 8 years ago by vorner

  • Owner changed from UnAssigned to vorner

comment:7 follow-up: Changed 8 years ago by vorner

  • Owner changed from vorner to jelte

Hello

Maybe the message should be „Dropping the connection and message“ instead of just message.

Should it have a changelog that the bug should be gone now?

I also noticed that the import pprint is not needed, there are only two commented-out calls to it. Should it be cleaned out?

And, finally, the branch does not compile for me. It fails in some dhcp-related C++ tests, even after merging with master. Should we wait for it to clean up? Do you have an idea if Tomek knows?

Thanks

comment:8 in reply to: ↑ 7 Changed 8 years ago by jelte

  • Owner changed from jelte to vorner

Replying to vorner:

Hello

Maybe the message should be „Dropping the connection and message“ instead of just message.

made it "dropping message and closing connection"

Should it have a changelog that the bug should be gone now?

ohyeah, how about:

  1. [defect] jelte

msgq no longer crashes if the remote end is closed while msgq tries to send data. It will now simply drop the message and close the connection itself.

I also noticed that the import pprint is not needed, there are only two commented-out calls to it. Should it be cleaned out?

removed

And, finally, the branch does not compile for me. It fails in some dhcp-related C++ tests, even after merging with master. Should we wait for it to clean up? Do you have an idea if Tomek knows?

luckily i missed this :) I'm assuming this has been fixed now, but either way this change should be orthogonal (except that perhaps now you haven't verified whether msgq now even runs at all)

comment:9 Changed 8 years ago by vorner

  • Owner changed from vorner to jelte

Thanks, this looks OK to merge.

comment:10 Changed 8 years ago by jelte

  • Resolution set to fixed
  • Status changed from reviewing to closed

Thanks, merged, closing ticket.

Note: See TracTickets for help on using tickets.