fiber: release lock in pool:put(conn) #55
Conversation
|
Please, add @ChangeLog for the patchset. |
|
Please give your branches a descriptive name according to SOP(for the future). |
5bfab0c to
6382545
Compare
Done. |
Ok. |
Because this test is not about pool connections, but my first patch is about. Also, this test does not fail on master, it was added, because Sanya (@Totktonada) requested it in the issue description. |
Optional!!! |
There was a problem hiding this comment.
Hi! As for me now the test seems not bad. I think the result could be something like this:
--- gh-34: Check that fiber is not blocked in the following case.
-- A connection conn is acquired from a pool and we acquire a
-- lock. Then we start the first fiber with pool:put(conn)`, which
-- try to acquire lock and yield. Then the second fiber is started
-- to perform request with `conn:execute()` which try to acquire
-- lock, because the connection is still marked as `usable` and
-- yield too. It's appeared that the lock is never released. Now,
-- it's fixed.
local function test_block_fiber_inf(test, pool)
test:plan(2)
local conn = pool:get()
local function fiber_block_request(conn)
local res, err = pcall(conn.execute, conn, 'SELECT "a"')
test:ok(res == false and string.find(err, 'Connection is broken'),
'query failed')
end
conn.queue:get()
fiber.create(pool.put, pool, conn)
local blocked_fiber = fiber.create(fiber_block_request, conn)
conn.queue:put(true)
local start_time = fiber.time()
local timeout = 5
local is_alive = true
-- Wait for the blocked fiber to finish,
-- but no more than "timeout" seconds.
while fiber.time() - start_time < timeout and is_alive do
fiber.sleep(0.1)
is_alive = blocked_fiber:status() ~= 'dead'
end
test:ok(is_alive == false, 'fiber is not blocked')
end
See some comments below.
I'm okay with your test version. I used it. |
|
I have no objections aside of naming and the forgotten print. I looked at the code and didn't look at the test again (I did it the past time). Please, rebase the patch upward master. That's all. I think we can push after that. |
|
Rebased. Let's push? |
|
Waiting for PR #58 to run tests. |
Make lock arithmetic proper and complite in conn:execute(), conn:close(), conn:reset() and conn:quote(). Release lock in pool:put(conn). The absence of this operation could lead to an infinite bloсking when used with fiber as describerd in the case below. A connection conn is acquired from a pool and we acquire a lock. Then we start the first fiber with pool:put(conn)`, which try to acquire lock and yield. Then the second fiber is started to perform conn:execute(), conn:reset() and conn:quote() which try to acquire lock, because the connection is still marked as `usable` and yield too. Before this patch, it's appeared that the lock is never released. Closes #34
|
Rebased on top of the latest master. |
@ChangeLog
pool:put(conn)andconn:execute()in different fibers (Ensure <connection object>:execute() don't block a fiber infinitely #34).