New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

common/admin_socket: close socket descriptor in destructor #4541

Merged
merged 1 commit into from May 8, 2015

Conversation

Projects
None yet
3 participants
@jbernard
Contributor

jbernard commented May 4, 2015

Long-running processes that do not reuse a single client connection will
see accumulating file descriptors as a result of not closing the
listening socket. In this case, eventually the system will reach
file-max and subsequent connections will fail.

Signed-off-by: Jon Bernard jbernard@tuxion.com

@jdurgin jdurgin self-assigned this May 4, 2015

@jdurgin

This comment has been minimized.

Member

jdurgin commented May 5, 2015

Looks right. The shutdown pipe is also leaking its read half - AdminSocket::entry() should close m_shutdown_rd_fd everywhere it returns.

Could you also add a bug to the ceph tracker and reference it in the commit message as "Fixes: #xxxxx"? That makes it easier to track backports. Thanks!

@jbernard

This comment has been minimized.

Contributor

jbernard commented May 7, 2015

I think the failure is caused by #4603 but I would like to move the thread join below the pipe close operations, so I'll push a rebase'd update once jenkins is unpaused.

Also, I just looked and it appears this code is duplicated in the OutputDataSocket class as well, and the bug is there too. Some code sharing is definately in order here and I'll tackle that next unless I'm off base.

@ghost

This comment has been minimized.

ghost commented May 7, 2015

Part of the fail is indeed #4603 but

FAIL: ./src/unittest_admin_socket

is probably yours.

@jbernard

This comment has been minimized.

Contributor

jbernard commented May 7, 2015

Yep, you're right, I'll get that fixed up as well.

common/admin_socket: close socket descriptor in destructor
Long-running processes that do not reuse a single client connection will
see accumulating file descriptors as a result of not closing the
listening socket.  In this case, eventually the system will reach
file-max and subsequent connections will fail.

Fixes: #11535

Signed-off-by: Jon Bernard <jbernard@tuxion.com>

jdurgin added a commit that referenced this pull request May 8, 2015

Merge pull request #4541 from jbernard/master
common/admin_socket: close socket descriptor in destructor

Reviewed-by: Josh Durgin <jdurgin@redhat.com>

@jdurgin jdurgin merged commit 640902e into ceph:master May 8, 2015

@jdurgin

This comment has been minimized.

Member

jdurgin commented May 8, 2015

Looks good! I don't see any reason not to share code with OutputDataSocket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment