Skip to content
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

Fix indefinite client hang after connection reset #29

Merged
merged 1 commit into from
May 15, 2019

Conversation

mrngm
Copy link
Contributor

@mrngm mrngm commented Apr 5, 2018

We discovered a problem in this library that causes the client to hang
indefinitely. This patch makes sure that the locking part
(net.textproto.Pipeline.StopRequest) is executed whenever the function
returns.

Steps to reproduce:

  1. Establish a normal connection with beanstalkd and keep the connection open
  2. Reserve a job through TubeSet.Reserve(...)
  3. Exit the server
  4. Reserve a job through TubeSet.Reserve(...). The expected error is:
    reserve-with-timeout: read tcp srcip:srcport->dstip:dstport: read: connection reset by peer
  5. Reserve a job through TubeSet.Reserve(...). The expected error is:
    reserve-with-timeout: write tcp srcip:srcport->dstip:dstport: write: broken pipe

After step 3, t.Conn.cmd(...) (in tubeset.go:30) is executed without error,
because writes to a half-closed socket are accepted. Step 4, then, fails with
the error mentioned. In step 5, the write fails and t.Conn.cmd(...) returns
early in conn.go:76.

However, c.c.StartRequest(r.id) is not properly ended and this causes jobs
with an id larger than r.id to wait indefinitely to be processed.

Cc: @Minnozz
Fixes: #21

We discovered a problem in this library that causes the client to hang
indefinitely. This patch makes sure that the locking part
(`net.textproto.Pipeline.StopRequest`) is executed whenever the function
returns.

Steps to reproduce:

1. Establish a normal connection with `beanstalkd` and keep the connection open
2. Reserve a job through `TubeSet.Reserve(...)`
3. Exit the server
4. Reserve a job through `TubeSet.Reserve(...)`. The expected error is:
`reserve-with-timeout: read tcp srcip:srcport->dstip:dstport: read: connection
reset by peer`
5. Reserve a job through `TubeSet.Reserve(...)`. The expected error is:
`reserve-with-timeout: write tcp srcip:srcport->dstip:dstport: write: broken
pipe`

After step 3, `t.Conn.cmd(...)` (in `tubeset.go:30`) is executed without error,
because writes to a half-closed socket are accepted. Step 4, then, fails with
the error mentioned. In step 5, the write fails and `t.Conn.cmd(...)` returns
early in `conn.go:76`.

However, `c.c.StartRequest(r.id)` is not properly ended and this causes jobs
with an `id` larger than `r.id` to wait indefinitely to be processed.

Cc: @Minnozz
Fixes: beanstalkd#21
@Minnozz
Copy link

Minnozz commented May 9, 2018

To summarize: The client will sometimes block indefinitely because of a bug where textproto.Conn.StartRequest() is called but textproto.Conn.EndRequest() is not if an error occurs somewhere in Conn.cmd(). This PR fixes that by deferring textproto.Conn.EndRequest().

@mrngm mrngm changed the title conn.go: execute c.c.EndRequest(r.id) when c.cmd(...) returns Fix indefinite client hang after connection reset Jan 23, 2019
@kr kr merged commit 390b03b into beanstalkd:master May 15, 2019
@kr
Copy link
Member

kr commented May 15, 2019

Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Graceful auto-reconnection
3 participants