On Wed, Sep 22, 2004 at 08:57:24PM -0600, Paul Querna wrote: > Attached is a Patch for apr_pollset_*. > Changes Include: > - Replace HAS_* with USE_* to remove some complex #ifdef stuff. > - Partially thread safe (*) > - Removes the limitation on the number of sockets that you can add to a > pollset (**)
This is kind of neat. But :) w.r.t thread-safety: Pushing this stuff down into the APR pollset code means that all non-threaded users of the pollset API incur extra overhead for functionality which they don't need or care about. In the event MPM, could you not, for instance, just keep two queues of "pollfds to remove" and "pollfds to add" which are handled with appropriate locking, and then process these queues calling _add and _remove appropriate after _poll() returns? i.e. ensure that a single thread owns the pollset structure. w.r.t. unlimited size of pollset: this could be solved in a general manner, for *all* implementations, by adding an apr_pollset_resize() call. So I'm worried this patch is really solving the problem in the wrong place. The thing you are exporting here in the APR API, "is the apr_pollset_* interface thread-safe in _add and _remove", seems really horrible. Either an API is thread-safe, or it isn't. Making that a *per-platform* flag seems like really bad design. joe
