daurnimator / lredis

A redis client for lua
MIT License
42 stars 7 forks source link

Prevent deadlocking due to errors #7

Open Oozlum opened 3 years ago

Oozlum commented 3 years ago

Prevent errors in protocol.read_response from propagating before the current request has been removed from the fifo. Otherwise all other pipelined requests will become deadlocked.

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-5.1%) to 79.245% when pulling d149fefa0b13a23303aca33542f8cbf34720e5ee on Oozlum:prevent_deadlock_on_error into 677cb3ed27219b2cf7e08290926c1769d08ae3f2 on daurnimator:master.

Oozlum commented 3 years ago

I think using a pcall here breaks under certain environments. I had to do a big workaround in lua-http to make pcall work in Lua 5.1:

https://github.com/daurnimator/lua-http/blob/54606a9d3fea9780468601ed0c8be48748e97204/http/util.lua#L210

Is there some other way we can solve this without a pcall?

Yes, actually, but the changes are a bit more invasive. I've committed a different implementation which uses a custom cqueues.socket:onerror handler and changes the internal error mechanism to use the 'return nil, error' convention rather than being assert() based. This allows handling of errors without using pcall.