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

feature: allow ngx.sleep to be used blockingly in non-yieldable phases #1759

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions README.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -5621,14 +5621,17 @@ ngx.sleep

**syntax:** *ngx.sleep(seconds)*

**context:** *rewrite_by_lua*, access_by_lua*, content_by_lua*, ngx.timer.*, ssl_certificate_by_lua*, ssl_session_fetch_by_lua**
**context:** *init_by_lua*, init_worker_by_lua*, set_by_lua*, rewrite_by_lua*, access_by_lua*, content_by_lua*, header_filter_by_lua*, body_filter_by_lua*, log_by_lua*, ngx.timer.*, balancer_by_lua*, ssl_certificate_by_lua*, ssl_session_fetch_by_lua*, ssl_session_store_by_lua*, exit_worker_by_lua**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not safe to use blocking sleeps in certain contexts like init_worker_by_lua*, set_by_lua*, header_filter_by_lua*, and etc.

I'm fine with enabling it in contexts where blocking operations do not matter, like in init_by_lua* and exit_worker_by_lua*. Otherwise it's too dangerous.


Sleeps for the specified seconds without blocking. One can specify time resolution up to 0.001 seconds (i.e., one millisecond).
Sleeps for the specified seconds without blocking in yieldable phases or blockingly in other phases.
One can specify time resolution up to 0.001 seconds (i.e., one millisecond).

Behind the scene, this method makes use of the Nginx timers.

Since the `0.7.20` release, The `0` time argument can also be specified.

Since the `FIXME` release, this method can be used in non-yieldable phases blockingly.

This method was introduced in the `0.5.0rc30` release.

[Back to TOC](#nginx-api-for-lua)
Expand Down
7 changes: 5 additions & 2 deletions doc/HttpLuaModule.wiki
Original file line number Diff line number Diff line change
Expand Up @@ -4720,14 +4720,17 @@ Since <code>v0.8.3</code> this function returns <code>1</code> on success, or re

'''syntax:''' ''ngx.sleep(seconds)''

'''context:''' ''rewrite_by_lua*, access_by_lua*, content_by_lua*, ngx.timer.*, ssl_certificate_by_lua*, ssl_session_fetch_by_lua*''
'''context:''' ''init_by_lua*, init_worker_by_lua*, set_by_lua*, rewrite_by_lua*, access_by_lua*, content_by_lua*, header_filter_by_lua*, body_filter_by_lua*, log_by_lua*, ngx.timer.*, balancer_by_lua*, ssl_certificate_by_lua*, ssl_session_fetch_by_lua*, ssl_session_store_by_lua*, exit_worker_by_lua*''

Sleeps for the specified seconds without blocking. One can specify time resolution up to 0.001 seconds (i.e., one millisecond).
Sleeps for the specified seconds without blocking in yieldable phases or blockingly in other phases.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'd better clarify the blockingly phases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and we need to highlight it's not recommended to use it in the non-yieldable phase since it will block the whole nginx worker process.

One can specify time resolution up to 0.001 seconds (i.e., one millisecond).

Behind the scene, this method makes use of the Nginx timers.

Since the <code>0.7.20</code> release, The <code>0</code> time argument can also be specified.

Since the <code>FIXME</code> release, this method can be used in non-yieldable phases blockingly.

This method was introduced in the <code>0.5.0rc30</code> release.

== ngx.escape_uri ==
Expand Down
23 changes: 17 additions & 6 deletions src/ngx_http_lua_sleep.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,34 @@ ngx_http_lua_ngx_sleep(lua_State *L)
return luaL_error(L, "attempt to pass %d arguments, but accepted 1", n);
}

r = ngx_http_lua_get_req(L);
if (r == NULL) {
return luaL_error(L, "no request found");
}

delay = (ngx_int_t) (luaL_checknumber(L, 1) * 1000);

if (delay < 0) {
return luaL_error(L, "invalid sleep duration \"%d\"", delay);
}

r = ngx_http_lua_get_req(L);
if (r == NULL) {
/* init_by_lua phase */
ngx_log_debug1(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0,
spacewander marked this conversation as resolved.
Show resolved Hide resolved
"lua ready to sleep for %d ms", delay);

ngx_msleep(delay);
return 0;
}

ctx = ngx_http_get_module_ctx(r, ngx_http_lua_module);
if (ctx == NULL) {
return luaL_error(L, "no request ctx found");
}

ngx_http_lua_check_context(L, ctx, NGX_HTTP_LUA_CONTEXT_YIELDABLE);
if (!(ctx->context & NGX_HTTP_LUA_CONTEXT_YIELDABLE)) {
ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
spacewander marked this conversation as resolved.
Show resolved Hide resolved
"lua ready to sleep for %d ms", delay);

ngx_msleep(delay);
return 0;
}

coctx = ctx->cur_co_ctx;
if (coctx == NULL) {
Expand Down
76 changes: 69 additions & 7 deletions t/077-sleep.t
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ log_level('debug');

repeat_each(2);

plan tests => repeat_each() * 71;
plan tests => repeat_each() * (blocks() * 4);

#no_diff();
no_long_string();
Expand Down Expand Up @@ -237,21 +237,20 @@ lua sleep timer expired: "/test?"



=== TEST 10: ngx.sleep unavailable in log_by_lua
=== TEST 10: ngx.sleep available in log_by_lua
--- config
location /t {
echo hello;
log_by_lua '
ngx.sleep(0.1)
';
log_by_lua_block {
ngx.sleep(0.001)
}
}
--- request
GET /t
--- response_body
hello
--- wait: 0.1
--- error_log
API disabled in the context of log_by_lua*
lua ready to sleep for 1 ms



Expand Down Expand Up @@ -500,3 +499,66 @@ f end
worker cycle
e?poll timer: 0
/



=== TEST 18: ngx.sleep(0) in no-yieldable phases
--- config
location /t {
echo hello;
log_by_lua_block {
ngx.sleep(0)
}
}
--- request
GET /t
--- response_body
hello
--- error_log
lua ready to sleep for 0 ms



=== TEST 19: ngx.sleep available in init_worker_by_lua
--- http_config
init_worker_by_lua_block {
local start = ngx.now()
ngx.sleep(0.1)
ngx.update_time()
package.loaded.gap = ngx.now() - start
}
--- config
location /t {
content_by_lua_block {
ngx.say(package.loaded.gap >= 0.1)
}
}
--- request
GET /t
--- no_error_log
[error]
--- response_body
true



=== TEST 20: ngx.sleep available in init_by_lua
--- http_config
init_by_lua_block {
local start = ngx.now()
ngx.sleep(0.1)
ngx.update_time()
package.loaded.gap = ngx.now() - start
}
--- config
location /t {
content_by_lua_block {
ngx.say(package.loaded.gap >= 0.1)
}
}
--- request
GET /t
--- no_error_log
[error]
--- response_body
true
12 changes: 5 additions & 7 deletions t/138-balancer.t
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use Test::Nginx::Socket::Lua;

repeat_each(2);

plan tests => repeat_each() * (blocks() * 4 + 9);
plan tests => repeat_each() * (blocks() * 4 + 8);

#no_diff();
no_long_string();
Expand Down Expand Up @@ -258,12 +258,12 @@ qr/\[error\] .*? failed to run balancer_by_lua\*: balancer_by_lua:2: API disable



=== TEST 10: ngx.sleep is disabled
=== TEST 10: ngx.sleep is allowed
--- http_config
upstream backend {
server 0.0.0.1;
balancer_by_lua_block {
ngx.sleep(0.1)
ngx.sleep(0.001)
}
}
--- config
Expand All @@ -272,10 +272,8 @@ qr/\[error\] .*? failed to run balancer_by_lua\*: balancer_by_lua:2: API disable
}
--- request
GET /t
--- response_body_like: 500 Internal Server Error
--- error_code: 500
--- error_log eval
qr/\[error\] .*? failed to run balancer_by_lua\*: balancer_by_lua:2: API disabled in the context of balancer_by_lua\*/
--- response_body_like: 502 Bad Gateway
--- error_code: 502



Expand Down
7 changes: 3 additions & 4 deletions t/142-ssl-session-store.t
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use File::Basename;

repeat_each(3);

plan tests => repeat_each() * (blocks() * 6 - 1);
plan tests => repeat_each() * (blocks() * 5 + 11);

$ENV{TEST_NGINX_HTML_DIR} ||= html_dir();

Expand Down Expand Up @@ -95,11 +95,11 @@ ssl_session_store_by_lua_block:1: ssl session store by lua is running!,



=== TEST 2: sleep is not allowed
=== TEST 2: sleep is allowed
--- http_config
ssl_session_store_by_lua_block {
local begin = ngx.now()
ngx.sleep(0.1)
ngx.sleep(0.001)
print("elapsed in ssl store session by lua: ", ngx.now() - begin)
}
server {
Expand Down Expand Up @@ -157,7 +157,6 @@ close: 1 nil

--- error_log
lua ssl server name: "test.com"
API disabled in the context of ssl_session_store_by_lua*

--- no_error_log
[alert]
Expand Down