On Fri, 4 Jun 2004, Justin Erenkrantz wrote:
> --On Friday, June 4, 2004 9:58 AM -0400 [EMAIL PROTECTED] wrote: > > > You don't need apr_*_mutex_join, that is what apr_*_mutex_child_init is > > supposed to do, but it can't, because the API doesn't work. The simple > > fact that you needed to insert a new method to do exactly what an existing > > method does proves that the API is currently broken. > > I think we disagree on what child_init should do then. I think child_init > should work when there is a shared address space. And, a proposed _join could > work when the address space is disjoint, but there is still a parent-child > relationship. I don't see why their functionality must be conflated. Because there is no reason to separate them. The two concepts are so inherently similar, that separating them just doesn't make sense. What controls if you need the features of child_init or join? It isn't the code that uses the lock, it is the lock mechanism. If you are using an anonymous lock mechanism, then you need the features in child_init, but if you are using a named lock mechanism, then you really want to use the features in join. It happens that child_init will work for named lock mechanisms as long as you used fork to create the second process, but that isn't due to design, it is a lucky hapenstance. What really dictates which you should use is the lock type. So, just change child_init to take the same parameters that you were going to pass to join, and do the right thing based on lock type. > And, > certainly, _create shouldn't be called twice (which is what testglobalmutex > was doing) - that would never work correctly as there needs to be only one > owner to the lock. Everybody agrees that the test was bogus. Will pointed it out when I committed the test, and I agreed with him then and now. But, don't say it would never work correctly, because that is actually the _only_ way the current code _can_ work when using apr_proc_create. Is it clean? No way, and it doesn't make any sense at all, but it does work. It just happens to work by chance instead of design. > > > Now, the reason that this patch doesn't actually fix the problem is that > > you haven't read my post about how to solve this problem. Some mutex > > types can't be shared without using fork. Unless you take that into > > account, your fix isn't correct. > > Yes, I read your earlier posts but they made no sense to me. Even with my > strawman patch, all of them would still require the use of fork for now. If > we wanted to relax that distinction, then we'd have to stop using anonymous > shared memory segments (like via IPC_PRIVATE). Yet, even if we did that, I > think the API itself (with the perhaps addition of _join) would be sufficient. > The fact we use tactics like IPC_PRIVATE are an implementation detail. (I'm > not sure it's worth adding this functionality though.) OK, this is where the problem is. Let me be abundantly clear. Any API that requires the use of fork ISN'T portable. It can't be, because fork doesn't exist on all of our platforms. This is why the current test code uses apr_proc_create, because that is how the code must work if it is to be portable. The only reason Apache hasn't hit this bug so far, is that we have a single child process in the Windows MPM, and thus no lock is needed. Try using the apr_lock types on Windows though, and they just don't work without re-creating the lock. That is completely bogus. Any API that is put forward needs to work on ALL systems equally well. Now, we do have some lock mechanisms that will _only_ work if you use fork(), ie anonymous lock mechanisms. The goal behind the concept I posted was to allow people to use anonymous lock mechanisms if they were certain they would only be on platforms that supported fork, but also provide a way to specify that you needed a named lock because this code was going to be used on platforms without fork, and thus it needed to support apr_proc_create. Ryan
