Date: Mon, 3 Mar 2025 17:20:57 +0000 From: Taylor R Campbell <riastr...@netbsd.org> Message-ID: <20250303172102.41b6b84...@mail.netbsd.org>
| It's not `wrong'; it's a legitimate way to eliminate unnecessary | failure modes, weighing costs of one approach vs costs of another. It is a very poor choice. | But it _is_ wrong for pthread_atfork to fail in pthread__init or | pthread_tsd_init. I'm not sure that should be the case - in practice (with a good implementation of pthread_atfork() which we don't quite have yet, but will) failure during any start time constructor should be an absurdly rare occurrence, if it does happen, lots more is going to fail later anyway (no available memory, even before the program has allocated anything!) so it wouldn't be unreasonable or the pthread init calls to simply fail as well, leaving a process in which threads cannot be created. | We can preallocate some pthread_atfork records (and use a custom | allocator with some nicely simplifying properties of the caller to | avoid malloc), but that doesn't guarantee anything as long as | applications ever call pthread_atfork in an ELF constructor. It is very difficult to absolutely guarantee anything. But assuming that a few pages of memory are available to be allocated (without the standard malloc) before any other allocations have been made, beyond what the constructors might need (they can't use malloc either) isn't unreasonable. Sure, it can fail, but pthread_atfork() callers should simply be taught to deal with that, somehow. Anything used on any non-NetBSD system would need to handle the possibility (or just ignore it, as we have largely been doing until now). Better than making things work in a way that can only possibly work on NetBSD. We're aiming for portable code, not lock in. | (b) doesn't actually create any compatibility constraints because | nothing outside libc uses it yet. yet being the operative word. | I'm happy to replace it by another signature that only has one extra | argument instead of three, A better model, if we were to go that way, and I really do not believe it is the correct way, would be to stick to 3 args (no extra ones needed) just make them be the storage spaces - with the desired function pointer already in place inside that struct. If you're going to make the struct public, you might as well use it. | We can define the interface so that the caller | has to allocate the larger of (say) eight aligned words per call or | space for three function pointers and two pointers, which is surely | enough to satisfy any reasonable implementation of fork. You can't guarantee that - we might want to add all kinds of things in the future, stats, timestamps (init time, last use time, ...), mutexes (or some other form of locking construct we haven't even invented yet). As you mentioned in some other message, essentially all of this is used when a fork() is happening, the costs of that will vastly outweigh anything nice we might want an atfork handler to also do. Nor can we just extend the struct here by using a free word as a pointer to extra memory allocated elsewhere, as the inability to allocate memory is the sole reason for desiring this new interface. Anything that makes any of that difficult is a poor design. Keeping all the info private to pthread_atfork() and fork() allows for anything we ever decide would be useful to be done. Further, this kind of thing is a programmer unsafe design, eventualy someone will allocate the storage they pass to libc_atfork or whatever a new interface would be called, on the (C) stack, or via malloc() and later free it, and havoc will result. kre