Live data from Hacker News

Tokio and Prctl = Nasty Bug

kobzol.github.io

21–30 of 78 posts

Re: Tokio and Prctl = Nasty Bug

#22
> In particular, it is not always possible for HQ to ensure that when a process that spawns tasks (called worker) quits unexpectedly (e.g. when it receives SIGKILL), its spawned tasks will be cleaned up. Sadly, Linux does not seem to provide any way of implementing perfect structured process management in user space. In other words, when a parent process dies, it is possible for its (grand)children to continue executing.

Uh, it does? It's called pid-namesp—

> There is a solution for this called PID namespaces,

I think maybe you've got the wrong idea about what "in user space" means? — processes running as root are still "in user space". The opposite of "user space" is "in the kernel".

> but it requires elevated privileges

I think that's only technically true. I believe you can unshare the PID namespace if you first unshare the user namespace — which causes the thing doing the unsharing of the user namespace to become "root" within that new namespace, and from there is permitted to unshare the pid namespace. I think: https://unix.stackexchange.com/a/672462/6013

I have no idea why that hoop has to be jumped through / I don't know what is being protected against by preventing unprivileged processes from making pid namespaces.

Whether or not that fits well with HQ's design … you'd have to be the judge of that.

There's also prctl(PR_SET_CHILD_SUBREAPER, ...)

Re: Tokio and Prctl = Nasty Bug

#23
If you want to be able to spawn processes that fask then `fork()` is NOT your friend. You want either `vfork()` (or `clone()` equivalent) or `posix_spawn()`.

`fork()` is inherently very slow due to the need to either copy the VM of the parent, or arrange to copy pages on write, or copy the resident set of the parent (then copy any pages paged-in when those page-in events happen -- all three of these options are very expensive.

Also, what I might recommend here is to create a `posix_spawn()`-like API that gives you asynchronous notification of the exec starting or failing, that way you don't block even for that. I'd use a `pipe()` that will be set to close on exec and which will therefore close when the exec starts, but if the exec fails I'd write the `errno` value into the pipe, that way EOF on the pipe implies the exec started while read on the pipe implies the exec failed and you can read the error number out of the pipe.

  https://gist.github.com/nicowilliams/a8a07b0fc75df05f684c23c18d7db234
  https://news.ycombinator.com/item?id=30502392
  https://github.com/famzah/popen-noshell

Re: Tokio and Prctl = Nasty Bug

#24
post #12

> It is called PR_SET_DEATHSIG, and we configure it when spawning tasks using the prctl syscall like this PDEATHSIG was to my knowledge (85% confidence) created for the original Linux userspace pthreads implementation (LinuxThreads¹, before NPTL) that was created back when it was implemented via kernel processes (the kernel had no concept of threads yet). This is AFAIK also why it behaves oddly in regards to later-ad…

There's been no fundamental change in the kernel level representation of pthreads, they are still clone()d processes with just some sharing flags set differently that eg affect how PIDs work.

Re: Tokio and Prctl = Nasty Bug

#25
post #13

Normally I'd stay away from job control posix APIs - but since HyperQueue is a job control system, it might be appropriate if the worker was a session leader. If it dies than all its subprocesses would receive SIGHUP - which is fatal by default. Generally you'd use this functionality to implement something like sshd or an interactive shell. HQ seems roughly analogous. https://notes.shichao.io/apue/ch9/#sessions

Yeah I suspect there may be a solution involving setsid

Re: Tokio and Prctl = Nasty Bug

#26
post #24
post #12

> It is called PR_SET_DEATHSIG, and we configure it when spawning tasks using the prctl syscall like this PDEATHSIG was to my knowledge (85% confidence) created for the original Linux userspace pthreads implementation (LinuxThreads¹, before NPTL) that was created back when it was implemented via kernel processes (the kernel had no concept of threads yet). This is AFAIK also why it behaves oddly in regards to later-ad…

There's been no fundamental change in the kernel level representation of pthreads, they are still clone()d processes with just some sharing flags set differently that eg affect how PIDs work.

More precisely, distinguishing a process and a thread is a pointless overspecification. Unfortunately POSIX mandates it and glibc accepts it.

If you want to register per-thread signal handlers you're forced to step outside the bounds of glibc and pthreads which I think is quite unfortunate.

Re: Tokio and Prctl = Nasty Bug

#27
post #14

Earlier quoted context omitted.

> PR_SET_PDEATHSIG automatically kills your children if you die, but unfortunately doesn’t extend to their descendants It indirectly does, unless you unset it the child dying will trigger another run of PDEATHSIG on the grandchildren, and so on. (The setting is retained across forks, as shown in the original article.)

> The setting is retained across forks, as shown in the original article That’s not what the man page says: > The parent-death signal setting is cleared for the child of a fork(2). https://man7.org/linux/man-pages/man2/pr_set_pdeathsig.2cons... Unless the man page is wrong?

I wonder if this is difference between libc fork (which calls clone syscall) and kernel fork syscall.

Re: Tokio and Prctl = Nasty Bug

#28
post #24

Earlier quoted context omitted.

There's been no fundamental change in the kernel level representation of pthreads, they are still clone()d processes with just some sharing flags set differently that eg affect how PIDs work.

More precisely, distinguishing a process and a thread is a pointless overspecification. Unfortunately POSIX mandates it and glibc accepts it. If you want to register per-thread signal handlers you're forced to step outside the bounds of glibc and pthreads which I think is quite unfortunate.

Digressing a little, but Glibc’s pthreads implementation is painful, because they don’t provide any public API to map a pthread_t to the kernel TID, except for the horrendously awful thread_db. Of course, for the current thread, you can just call gettid() - but if you want to map pthread_t to TID for another thread, the thread_db abomination is the only supported way. Bionic supplies a nice simple pthread_gettid_np() for this, macOS has that too (albeit sadly with an incompatible prototype).

Now, pthread_t is actually a pointer to an undocumented structure, and the TID is stored at a certain offset in it… so it is easy to pull the TID from there. Until some day the glibc developers change the layout of the structure and suddenly that code breaks.

There’s an entry in glibc’s bug tracker for this - https://sourceware.org/bugzilla/show_bug.cgi?id=27880 - but it doesn’t look like it will be implemented any time soon

Re: Tokio and Prctl = Nasty Bug

#29

If you want to be able to spawn processes that fask then `fork()` is NOT your friend. You want either `vfork()` (or `clone()` equivalent) or `posix_spawn()`. `fork()` is inherently very slow due to the need to either copy the VM of the parent, or arrange to copy pages on write, or copy the resident set of the parent (then copy any pages paged-in when those page-in events happen -- all three of these options are very…

Copying the page table isn't free but it isn't particularly expensive either. At least unless the parent is a real behemoth.

Unless you enjoy footguns posix_spawn is probably a better idea than vfork. (Unless you actually need vfork of course.)

The async pipe idea sounds interesting but I'm not clear how it would work. It seems like you'd have to use vfork to implement it but vfork is blocking until you call exec so doesn't that defeat the purpose?

Re: Tokio and Prctl = Nasty Bug

#30

I may be mistaken, but I believe the bug still exists, but in a more esoteric manner; and a future change might cause the bug to exist again. The author might want to warn against usage of `tokio::task::block_in_place`, if the underlying issue can't be fixed. The reason the current approach works is it runs on tokio's worker threads, which last the lifetime of the tokio runtime. However, if `tokio::task::block_in_pla…

That's a very good point! But yeah, we use the single threaded runtime, so this shouldn't be a concern.
Post reply on HN