3:02 a.m.
Hi,
I've taken a look at lsignal and implemented some basic signal handling
using the debug hook method that it uses.
Luxio also blocks signals while they're being handled on the Lua side.
I'm not sure why lsignal doesn't do this, it seems necessary?
Thanks,
Richard
Richard Ipsum (1):
Bind sigaction(2)
luxio.c | 82 +++++++++++++++++++++++++++++++++++++++-
tests/test-sigaction.lua | 39 +++++++++++++++++++
2 files changed, 120 insertions(+), 1 deletion(-)
create mode 100644 tests/test-sigaction.lua
--
2.19.1
3:02 a.m.
New subject: [PATCH 1/1] Bind sigaction(2)
---
luxio.c | 82 +++++++++++++++++++++++++++++++++++++++-
tests/test-sigaction.lua | 39 +++++++++++++++++++
2 files changed, 120 insertions(+), 1 deletion(-)
create mode 100644 tests/test-sigaction.lua
diff --git a/luxio.c b/luxio.c
index 1dca902..0f48dcf 100644
--- a/luxio.c
+++ b/luxio.c
@@ -481,7 +481,85 @@ luxio_kill(lua_State *L) /* 3.3.2 */
/* Signals are going to be almost impossible to do nicely and safely. */
/* TODO: Manipulate Signal Sets 3.3.3 */
-/* TODO: sigaction() 3.3.4 */
+
+static struct {
+ lua_State *state;
+ lua_Hook orighook;
+ int orighookmask;
+ int orighookcount;
+ sigset_t origset;
+ int signo;
+ int handlers[512];
+} luxio__signal_ctx;
+
+static void luxio__sigaction_hook(lua_State *L, lua_Debug *ar)
+{
+ /* Push the callback onto the stack using the Lua reference we */
+ /* stored in the registry */
+ lua_rawgeti(L, LUA_REGISTRYINDEX,
+ luxio__signal_ctx.handlers[luxio__signal_ctx.signo]);
+ lua_pushinteger(L, luxio__signal_ctx.signo);
+
+ lua_pcall(L, 1, 0, 0);
+
+ /* Restore original hook */
+ lua_sethook(L, luxio__signal_ctx.orighook,
+ luxio__signal_ctx.orighookmask,
+ luxio__signal_ctx.orighookcount);
+
+ /* The signal we're currently handling was blocked during signal delivery,
+ * but we also blocked everything manually in the handler,
+ * so we must now unblock this signal ourselves as well.
+ */
+ sigdelset(&luxio__signal_ctx.origset, luxio__signal_ctx.signo);
+
+ /* Restore the original signal mask */
+ sigprocmask(SIG_SETMASK, &luxio__signal_ctx.origset, NULL);
+}
+
+static void luxio__sigaction_handler(int signo)
+{
+ sigset_t set;
+ lua_State *L = luxio__signal_ctx.state;
+
+ /* Block everything till we're done handling the signal on the lua side */
+ sigfillset(&set);
+ sigprocmask(SIG_SETMASK, &set, &luxio__signal_ctx.origset);
+
+ luxio__signal_ctx.orighook = lua_gethook(L);
+ luxio__signal_ctx.orighookmask = lua_gethookmask(L);
+ luxio__signal_ctx.orighookcount = lua_gethookcount(L);
+ luxio__signal_ctx.signo = signo;
+
+ lua_sethook(L, luxio__sigaction_hook,
+ LUA_MASKCALL | LUA_MASKRET | LUA_MASKCOUNT, 1);
+}
+
+static int
+luxio_sigaction(lua_State *L)
+{
+ int signo;
+ struct sigaction sa = {
+ .sa_handler = luxio__sigaction_handler
+ };
+
+ signo = luaL_checkinteger(L, 1);
+
+ if (!lua_isfunction(L, 2)) {
+ lua_pushinteger(L, -1);
+ lua_pushinteger(L, EINVAL);
+ return 2;
+ }
+
+ luxio__signal_ctx.handlers[signo] = luaL_ref(L, LUA_REGISTRYINDEX);
+ luxio__signal_ctx.state = L;
+
+ lua_pushinteger(L, sigaction(signo, &sa, NULL));
+ lua_pushinteger(L, errno);
+
+ return 2;
+}
+
/* TODO: pthread_sigmask(), sigprocmask() 3.3.5 */
/* TODO: sigpending() 3.3.6 */
/* TODO: sigsuspend() 3.3.7 */
@@ -4563,6 +4641,8 @@ luxio_functions[] = {
{ "chdir", luxio_chdir },
{ "getcwd", luxio_getcwd },
+ { "sigaction", luxio_sigaction },
+
{ "alarm", luxio_alarm },
{ "pause", luxio_pause },
{ "sleep", luxio_sleep },
diff --git a/tests/test-sigaction.lua b/tests/test-sigaction.lua
new file mode 100644
index 0000000..bfd6af1
--- /dev/null
+++ b/tests/test-sigaction.lua
@@ -0,0 +1,39 @@
+local os = require "os"
+local l = require "luxio"
+local io = require "io"
+
+function MyCallback(signo)
+ print("Hello World!", signo)
+end
+
+function MyGoodbyeCallback(signo)
+ print("Goodbye cruel world!", signo)
+end
+
+function MySIGTERMCallback(signo)
+ print("SIGTERM !!!!", signo)
+ os.exit(os.EXIT_SUCCESS)
+end
+
+r, errno = l.sigaction(l.SIGINT, MyCallback)
+if r == -1 then
+ io.stderr:write(("sigaction: %s\n"):format(l.strerror(errno)))
+ os.exit(os.EXIT_FAILURE)
+end
+
+r, errno = l.sigaction(l.SIGINT, MyGoodbyeCallback)
+if r == -1 then
+ io.stderr:write(("sigaction: %s\n"):format(l.strerror(errno)))
+ os.exit(os.EXIT_FAILURE)
+end
+
+r, errno = l.sigaction(l.SIGTERM, MySIGTERMCallback)
+if r == -1 then
+ io.stderr:write(("sigaction: %s\n"):format(l.strerror(errno)))
+ os.exit(os.EXIT_FAILURE)
+end
+
+while (true) do
+ print("sleep!")
+ l.sleep(2)
+end
--
2.19.1
3:36 a.m.
On Sun, Apr 21, 2019 at 07:02:06PM +0100, Richard Ipsum wrote:
Hi,
I've taken a look at lsignal and implemented some basic signal handling
using the debug hook method that it uses.
Luxio also blocks signals while they're being handled on the Lua side.
I'm not sure why lsignal doesn't do this, it seems necessary?
bounce
Thanks,
Richard
Richard Ipsum (1):
Bind sigaction(2)
luxio.c | 82 +++++++++++++++++++++++++++++++++++++++-
tests/test-sigaction.lua | 39 +++++++++++++++++++
2 files changed, 120 insertions(+), 1 deletion(-)
create mode 100644 tests/test-sigaction.lua
--
2.19.1
4:07 p.m.
Firstly, I apologise. I have had this on my list to look at for so long
that I must have become blind to it.
On Sun, Apr 21, 2019 at 19:02:06 +0100, Richard Ipsum wrote:
I've taken a look at lsignal and implemented some basic signal
handling
using the debug hook method that it uses.
Cool.
Luxio also blocks signals while they're being handled on the Lua
side.
I'm not sure why lsignal doesn't do this, it seems necessary?
I'm unsure that it's worthwhile blocking the signals in this fashion,
though I suppose it saves either losing signals or having to queue them
in luxio rather than in the kernel.
I'll bear this design decision in mind when looking over the code which
I'll do now.
D.
--
Daniel Silverstone http://www.digital-scurf.org/
PGP mail accepted and encouraged. Key Id: 3CCE BABE 206C 3B69
4:18 p.m.
New subject: [PATCH 1/1] Bind sigaction(2)
On Sun, Apr 21, 2019 at 19:02:07 +0100, Richard Ipsum wrote:
diff --git a/luxio.c b/luxio.c
+
+static struct {
+ lua_State *state;
+ lua_Hook orighook;
+ int orighookmask;
+ int orighookcount;
+ sigset_t origset;
+ int signo;
+ int handlers[512];
Is 512 a defined limit on handlers, or is this a magic number you've derived
from somewhere else? If the latter, can it be a define (LUXIO_MAX_SIGNAL) or
somesuch?
+} luxio__signal_ctx;
Currently this approach means that only one Lua VM in a given process could
have signal handlers registered -- I'm "okay" with that, but it's worth
documenting the limitation.
+static void luxio__sigaction_handler(int signo)
Is there value in being an `sa_sigaction` handler and saving / passing-to-lua
any of the siginfo_t ? If not, no worries.
+static int
+luxio_sigaction(lua_State *L)
Documentation comment for this please, so that the generated docs can
explain both what a signal is, what the limitations are on it, etc.
+{
+ int signo;
+ struct sigaction sa = {
+ .sa_handler = luxio__sigaction_handler
+ };
+
+ signo = luaL_checkinteger(L, 1);
This number can end up out of the range of valid signals (e.g. less than one
or more than your 512 above).
+
+ if (!lua_isfunction(L, 2)) {
+ lua_pushinteger(L, -1);
+ lua_pushinteger(L, EINVAL);
+ return 2;
+ }
Would it make sense to support nil in this position to mean "deregister handler"
?
---
This looks like a reasonable approach over-all and I'd support merging it with
the above concerns resolved (such as deregistering handlers, dealing with the
limits, etc).
Thank you, and again I apologise for taking so long to respond.
D.
--
Daniel Silverstone http://www.digital-scurf.org/
PGP mail accepted and encouraged. Key Id: 3CCE BABE 206C 3B69
5:41 p.m.
New subject: [PATCH 1/1] Bind sigaction(2)
On Fri, May 24, 2019 at 08:18:22AM +0100, Daniel Silverstone wrote:
On Sun, Apr 21, 2019 at 19:02:07 +0100, Richard Ipsum wrote:
> diff --git a/luxio.c b/luxio.c
> +
> +static struct {
> + lua_State *state;
> + lua_Hook orighook;
> + int orighookmask;
> + int orighookcount;
> + sigset_t origset;
> + int signo;
> + int handlers[512];
Is 512 a defined limit on handlers, or is this a magic number you've derived
from somewhere else? If the latter, can it be a define (LUXIO_MAX_SIGNAL) or
somesuch?
The platform I'm using defines NSIG to be 33. NSIG sadly isn't in POSIX,
on Linux NSIG is 65, since it's not portable I just picked a number
that's much larger than either of these, it wastes memory but wasting
a few ints to gain the convenience of an array seems fine to me.
I agree that it should be defined in a constant.
> +} luxio__signal_ctx;
Currently this approach means that only one Lua VM in a given process could
have signal handlers registered -- I'm "okay" with that, but it's
worth
documenting the limitation.
I hadn't thought about there being more than one VM, I'll try and have
a think about how best to address this.
> +static void luxio__sigaction_handler(int signo)
Is there value in being an `sa_sigaction` handler and saving / passing-to-lua
any of the siginfo_t ? If not, no worries.
I'd like to do that as well, just started with the basics to see if the
approach is reasonable.
> +static int
> +luxio_sigaction(lua_State *L)
Documentation comment for this please, so that the generated docs can
explain both what a signal is, what the limitations are on it, etc.
Ack
> +{
> + int signo;
> + struct sigaction sa = {
> + .sa_handler = luxio__sigaction_handler
> + };
> +
> + signo = luaL_checkinteger(L, 1);
This number can end up out of the range of valid signals (e.g. less than one
or more than your 512 above).
Good point
> +
> + if (!lua_isfunction(L, 2)) {
> + lua_pushinteger(L, -1);
> + lua_pushinteger(L, EINVAL);
> + return 2;
> + }
Would it make sense to support nil in this position to mean "deregister
handler" ?
Yeah I think that would be a good idea.
---
This looks like a reasonable approach over-all and I'd support merging it with
the above concerns resolved (such as deregistering handlers, dealing with the
limits, etc).
Thank you, and again I apologise for taking so long to respond.
Thanks for the feedback!
Richard
10:37 p.m.
On Fri, May 24, 2019 at 08:07:20AM +0100, Daniel Silverstone wrote:
Firstly, I apologise. I have had this on my list to look at for so
long
that I must have become blind to it.
On Sun, Apr 21, 2019 at 19:02:06 +0100, Richard Ipsum wrote:
> I've taken a look at lsignal and implemented some basic signal handling
> using the debug hook method that it uses.
Cool.
> Luxio also blocks signals while they're being handled on the Lua side.
> I'm not sure why lsignal doesn't do this, it seems necessary?
I'm unsure that it's worthwhile blocking the signals in this fashion,
though I suppose it saves either losing signals or having to queue them
in luxio rather than in the kernel.
I've been thinking about this and it seems absolutely necessary.
If you don't block them while you're handling then there's nothing to
stop another signal coming in and clobbering everything while you're
still trying to handle the first one. You can queue them yourself
but nothing stops a signal being delivered while you're performing
operations on your queue, seems safer to let the OS do the queueing.
I've been implementing a possible solution to avoid the limit of one
VM but I have to say that I don't like it, it seems a lot more fragile
to me and so I'm wondering whether there is really any compelling
reason that Luxio would worry about supporting handlers set by
multiple VMs? If not then maybe we just document this limitation?
Thanks,
Richard
4:30 a.m.
On Tue, May 28, 2019 at 14:37:17 +0100, Richard Ipsum wrote:
> I'm unsure that it's worthwhile blocking the signals in
this fashion,
> though I suppose it saves either losing signals or having to queue them
> in luxio rather than in the kernel.
I've been thinking about this and it seems absolutely necessary.
If you don't block them while you're handling then there's nothing to
stop another signal coming in and clobbering everything while you're
still trying to handle the first one. You can queue them yourself
but nothing stops a signal being delivered while you're performing
operations on your queue, seems safer to let the OS do the queueing.
Fair, I accept your reasoning.
I've been implementing a possible solution to avoid the limit of
one
VM but I have to say that I don't like it, it seems a lot more fragile
to me and so I'm wondering whether there is really any compelling
reason that Luxio would worry about supporting handlers set by
multiple VMs? If not then maybe we just document this limitation?
As I (think) I said before - I'm okay with the limitation as long as it is
clearly documented, and ideally protected against (i.e. if you attempt to
register a signal handler from a Lua state which isn't the one cached
in the global context, then you get an error return from the function call)
Does that sound okay to you?
D.
--
Daniel Silverstone http://www.digital-scurf.org/
PGP mail accepted and encouraged. Key Id: 3CCE BABE 206C 3B69
4:47 a.m.
On Tue, May 28, 2019 at 08:30:49PM +0100, Daniel Silverstone wrote:
On Tue, May 28, 2019 at 14:37:17 +0100, Richard Ipsum wrote:
> > I'm unsure that it's worthwhile blocking the signals in this fashion,
> > though I suppose it saves either losing signals or having to queue them
> > in luxio rather than in the kernel.
>
> I've been thinking about this and it seems absolutely necessary.
> If you don't block them while you're handling then there's nothing to
> stop another signal coming in and clobbering everything while you're
> still trying to handle the first one. You can queue them yourself
> but nothing stops a signal being delivered while you're performing
> operations on your queue, seems safer to let the OS do the queueing.
Fair, I accept your reasoning.
> I've been implementing a possible solution to avoid the limit of one
> VM but I have to say that I don't like it, it seems a lot more fragile
> to me and so I'm wondering whether there is really any compelling
> reason that Luxio would worry about supporting handlers set by
> multiple VMs? If not then maybe we just document this limitation?
As I (think) I said before - I'm okay with the limitation as long as it is
clearly documented, and ideally protected against (i.e. if you attempt to
register a signal handler from a Lua state which isn't the one cached
in the global context, then you get an error return from the function call)
Does that sound okay to you?
Yeah, I'll make those changes then.
Thanks,
Richard
1580
days inactive
1617
days old
8 comments
2 participants
participants (2)
-
Daniel Silverstone -
Richard Ipsum