Skip to content

Added a thread killing API #288

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 22 commits into from
Closed

Added a thread killing API #288

wants to merge 22 commits into from

Conversation

aviramc
Copy link

@aviramc aviramc commented Oct 3, 2013

Added ngx.thread.kill that gets a list of Lua threads and kills them.
This includes the comments by @agentzh to the cleanup of the threads.
I've also removed the function ngx.thread.kill_sleeping, and it seems to be not useful anymore.

The is a part of pull request #286

Aviram and others added 13 commits April 24, 2013 10:36
Added an optional parameter for ngx.req.set_header and ngx.req.clear_…
3f39535 
…header that determines whether or not to replace underscores with hyphens.

Previously, underscores were replaced unconditionally.
Currently each of the functions has another boolean argument. If it's false, underscores would not be touched. If it's true, they would.
The default value of the argument is true.
Merged with 0.8.1
886e27b
Changed the replace_underscores parameter to a table of parameters (o…
1c5e5b9 
…ptions), in which the only possible option currently is replace_underscores.
Added an options table to clean_header as well.
f2e849a
Merge branch 'master' of https://github.com/chaoslawful/lua-nginx-module
e1281a2
Removed unneeded variable.
d4d2988
Added two killing functions -
2a84a4c 
 ngx.thread.kill - kills a thread.
 ngx.thread.kill_sleeping - either kills immediately a sleeping thread, or, if the thread is not sleeping, can wait for it to end. Useful for a thread that has a loop in which it sleeps.
del_thread doesn't remove the sleeping timer, which may cause segfaul…
7cd287a 
…ts when using ngx.thread.kill_sleeping.

Now removing the timer.
Merge branch 'master' of https://github.com/chaoslawful/lua-nginx-module
e699fcc
Corrected the killing of a thread due to a useful review by @agentzh
0a6a890 
It seems that this makes kill_sleeping unnecessary, so it was removed.
Reverted unnecessary changes.
e9e1dec
Reverted unnecessary changes.
24cb685
Reverted unnecessary changes.
571d777
@agentzh
Copy link
Member

agentzh commented Oct 30, 2013

Several comments on your latest patch:

  1. I think it is better to return the number of threads that are successfully killed. We can make ngx_http_lua_del_thread() return an ngx_int_t value like NGX_OK and NGX_DECLINED.
  2. When ngx_http_lua_get_co_ctx() returns NULL, it means the thread is already dead (and waited), we can just skip such threads without throwing out a Lua error.
  3. We should not set sub_coctx->co_status = NGX_HTTP_LUA_CO_DEAD in ngx_http_lua_uthread_kill() because ngx_http_lua_del_thread() already does that for us.
  4. We should skip running threads with pending subrequests. It is never safe to kill such threads.
  5. It will be great if you can add some test cases for this new feature to exercise various cases.

aviram added 8 commits December 10, 2013 16:01
Merge remote-tracking branch 'original-repository/master'
974112f
Merge branch 'master' into thread-kill
5d9ac66
ngx_http_lua_del_thread now returns rc
5d52ad2
Some fixes according to remarks by @agentZ :
16616b3 
 - Added a return value to ngx_http_lua_del_thread(), and checking the return value in the thread killing function.
 - When ngx_http_lua_get_co_ctx() returns NULL, this now means that the thread is already dead, no need in trying to kill it.
 - Returning the number of the threads killed successfully.
 - Not killing threads that have pending subrequests.
Merge branch 'master' of https://github.com/chaoslawful/lua-nginx-module
48fdf3d
Merge branch 'master' into thread-kill
a2f7b55
Merge branch 'master' of https://github.com/chaoslawful/lua-nginx-module
305f0a5
Merge branch 'master' into thread-kill
ec7b791
@aviramc
Copy link
Author

aviramc commented Dec 24, 2013

@agentzh this commit f649935 means that ngx_http_lua_get_co_ctx() doesn't return NULL anymore when a thread is dead. It's okay, but I want you to notice that it affects my patch. I'll change the way in which I check if the thread is dead.

@agentzh
Copy link
Member

agentzh commented Dec 24, 2013

@aviramc Yes, you're right. And I need update the uthread.c file in my current master branch as well :)

agentzh added a commit that referenced this pull request Jun 21, 2014
@agentzh
feature: added new API function ngx.thread.kill() for killing a user …
9bdda56 
…"light thread". thanks aviramc for the original patch in #288.
@agentzh
Copy link
Member

agentzh commented Jun 21, 2014

@aviramc Sorry for the long delay on my side!

Will you review and try out the following modified version of your patch on your side?

9bdda56

Thanks!

@agentzh
Copy link
Member

agentzh commented Jun 21, 2014

@aviramc BTW, I removed the multi-argument support because it makes JIT compiling impossible for FFI-based implementations.

@aviramc
Copy link
Author

aviramc commented Jun 22, 2014

@agentzh , it seems that 9bdda56 doesn't refer to the case in which a thread has pending subrequests. Other than that (the multiple to single argument change is fine by me), looks great.

@agentzh
Copy link
Member

agentzh commented Jun 22, 2014

@aviramc Nice catch! Some how I missed that :P Will fix it :)

agentzh added a commit that referenced this pull request Jun 26, 2014
@agentzh
feature: added new API function ngx.thread.kill() for killing a user …
b5f838f 
…"light thread". thanks aviramc for the original patch in #288.
@agentzh
Copy link
Member

agentzh commented Jun 26, 2014

@aviramc I've included the pending subrequest check in the new commit 6885462. Will you have another look? ;) Thanks!

agentzh added a commit that referenced this pull request Jun 26, 2014
@agentzh
feature: added new API function ngx.thread.kill() for killing a user …
6885462 
…"light thread". thanks aviramc for the original patch in #288.
@agentzh
Copy link
Member

agentzh commented Jul 1, 2014

This feature has already been included in the v0.9.9 release of ngx_lua. I'm closing this.

@agentzh agentzh closed this Jul 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aviramc @agentzh