Hallvard Breien Furuseth wrote: > On 14/06/2018 20:34, Howard Chu wrote: >> Hallvard Breien Furuseth wrote: >>> Wait, what? The child didn't write any pids to the table. If >>> there are pids for it to clear, something is wrong. That can be: >> >> No, there most likely aren't PIDs for it to clear. But most importantly, it >> must not clear using env->me_pid since that is still the parent's PID. >> >>> - The pid is from an older run, and the system reused the pid. If we >>> code for this, it should be to clear such pids during mdb_env_open. >>> It gets very arbitrary to clear them when the user does env_close() >>> in the child and the child happens to have the that reused pid. >> >> No. If the system reused the PID then that means the previous owner of that >> PID is dead already anyway. > > Right... > >> If there are any table entries under that PID there's no problem removing >> them. > > The problem is that it's arbitrary. Something is exiting without > clearing its pid slots, and 0.001% of the time this code will catch it.
Sure. But I don't think it's worthwhile to make a special case here to skip this step. > Better to skip this cleanup, or to always do it - but "always" belongs > in env_open(). If a new env is seeing its pid in use, then it can safely > clean it out. Last we talked about that, I liked it and you > did not:-) And I don't see any particular reason to add this step. >>> - The child opened a new MDB_env with the same path, and closes the >>> parent's env afterwards. Don't Do That, users. But what to do in >>> this situation, if we are to have code for it, is to do nothing with >>> the lockfile. Not close it either, since closing it will lose the >>> lock for the env which was opened in the child. >> >> That's a completely different case. This ITS is specifically about a process >> with an open environment that subsequently forks. > > No, I mean: > > Parent, pid P: > open env A. > > Child, pid Q: > - open and start using env B with the same path. > - close env A, while B is still open. > With the new code, this clears pid Q - i.e. B's pids. > But lmdb.h already lists that as a "don't do that", > because it also removes B's lock on the lockfile. > Yeah, this is still a "don't do that" though, regardless of this particular ITS. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
