v0.19: port std/os signals #1484
Labels
No labels
UX
active development
backlog
blocker
bootstrap
bounty
bug
dependencies
discussion
documentation
duplicate
enhancement
flaky test
help wanted
invalid
javascript
question
release
tendentious
wontfix
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
mighty-gerbils/gerbil!1484
Loading…
Reference in a new issue
No description provided.
Delete branch "jay/gerbil:v0.19-signal"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Ports the remaining signal-handling modules to v0.19 under
:std/os, includingsignal,signalfd,signal-handler, andkqueue.WIP: v0.19: port std/os signalsto v0.19: port std/os signals0583d874a9to05eb73415athis looks impressively good, but i'll have to do a thorough pass. Spotted an issue at first pass.
Have you run the tests in bsd/darwin? I only have linux boxes so i can't verify those tests.
@ -0,0 +90,4 @@(let (count (device-read device buffer 0 (u8vector-length buffer)))(cond((fxnegative? count)(device-wait-input! device !NoTimeout))shouldn't we loop back after device-wait-input! return. missing a (wait-loop) call.
wait-loop is on line 107, outside the cond
@vyzo wrote in #1484 (comment):
My work computer is a mac, lemme try it there. will report back
Looks overall good to me, but I have a few concerns that I believe deserve comments in the code at least—if not in this PR, in a quick follow-up.
@ -0,0 +13,4 @@./signalfd)(export signalfd-test)(def signalfd-testMy clanker friend GPT 5.6 Sol (High) says:
This test isn't SMP-safe as currently written.
sigprocmaskonly blocks SIGHUP in the calling thread, whilekill(getpid(), SIGHUP)may deliver it to any unblocked runtime thread; with the default disposition this can simply kill gxtest. 0583d874a9 seems to have fixed exactly this by blocking before fork/exec, but that commit disappeared in the force-push. Was that intentional? I think we need that setup (or equivalent) back.@ -0,0 +70,4 @@(:- device OSDevice)write-fd(make-mutex 'signal-handler))(spawn/name 'signal-handler signal-handler-wait self)Clanker says:
Of course, to make things more interesting, the situation will be different with green threads vs actual SMP. At least a TODO is required. @vyzo care to comment?
i agree, this is problematic. this needs to run on all threads, with on-all-processors that used to be in the now defunct std/misc/threads. I think we need to ressurect this in std/sync/smp and have the on-all-processors procedure there.
we can also do with a TODO here, but i would prefer to resolve this as it is a real issue.
seems we are going to ressurect some more stuff from that module as i am working on porting the remaining on std (std/net/repl uses some). i will put them in std/sync/threads, we can leave a big fat TODO in this pr for now.
@ -0,0 +359,4 @@(:- (kevent-ident events i) :fixnum))))(when event-handler(signal-handler-dispatch(:- event-handler :procedure)))))Clanker says (after I corrected its suggestion):
@ -0,0 +381,4 @@((vector-ref handlers signo)(vector-set! handlers signo thunk))(else(ignore-signal! sh signo)Clanker friend says of FreeBSD (as contrasted to other BSDs):
I'd say this deserves at the very least a "TODO" comment. And I don't think no-op should be it—some kind of wait() and handling of subprocess record would be more appropriate—through a user-overridable hook, I suppose.
@vyzo what is the follow-up plan? Create an issue? Add a - [ ] box to an existing issue/TODO ? To be done immediately by your clanker?
let's create an issue so that we don't forget. SMP will land in the not so distant future.