Set MSG_NOSIGNAL for UnixStream

[mlowicki]

#139956

Same logic as for

rust/library/std/src/sys/net/connection/socket.rs

Lines 399 to 405 in 1f76d21

	pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
	let len = cmp::min(buf.len(), <wrlen_t>::MAX as usize) as wrlen_t;
	let ret = cvt(unsafe {
	c::send(self.inner.as_raw(), buf.as_ptr() as *const c_void, len, MSG_NOSIGNAL)
	})?;
	Ok(ret as usize)
	}

.

[rustbot]

r? @joboet

rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[joboet]

@rustbot label +I-libs-nominated

Hey T-libs! Was there a particular reason for this? I think it's reasonable to say that generating signals doesn't make sense for everything but console I/O.

r=me if you agree

[joshtriplett]

Nominating for libs-api; I think the handling of broken pipes is an API question.

[joshtriplett]

In general, ignoring SIGPIPE and producing EPIPE is often a good idea. However, I'm not sure if this change is something we can or should do on behalf of the user without knowing what they want, for multiple reasons.

The minor reason is that the user might not be expecting EPIPE. However, I think that's less of a problem for things other than stdout/stderr.

The larger reason is that switching from write to send could break users who convert something other than a socket to a UnixStream, because send doesn't work on non-sockets.

I think it's still reasonable to consider this change, but we're going to want to announce it as a potential compatibility issue.

[mlowicki]

In general, ignoring SIGPIPE and producing EPIPE is often a good idea. However, I'm not sure if this change is something we can or should do on behalf of the user without knowing what they want, for multiple reasons.

The minor reason is that the user might not be expecting EPIPE. However, I think that's less of a problem for things other than stdout/stderr.

The larger reason is that switching from write to send could break users who convert something other than a socket to a UnixStream, because send doesn't work on non-sockets.

I think it's still reasonable to consider this change, but we're going to want to announce it as a potential compatibility issue.

FTR MSG_NOSIGNAL is set for e.g. UnixDatagram -

rust/library/std/src/os/unix/net/datagram.rs

Line 518 in 1f76d21

MSG_NOSIGNAL,

and documentation also doesn't mention that fact so might be worthy to improve API docs too.

[joshtriplett]

@mlowicki Agreed.

[joshtriplett]

We discussed this in today's @rust-lang/libs-api call. We're fine with doing this, but we'd like to do an FCP to confirm this change, since it's user-visible (albeit not a documented guarantee).

@rfcbot merge

[rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[mlowicki]

@mlowicki Agreed.

Once this gets merged I can create another PR to improve the doc for UnixDatagram and UnixStream.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[tgross35]

Thank you!

@bors r+

[bors]

📌 Commit 426ab14 has been approved by tgross35

It is now in the queue for this repository.