After hours of debugging, the answer was -trivial-, but extremely
obscure and difficult to trace. This should do away with the handle
corruption we experienced. If anyone cares to walk the crt (source
is installed in vc\src\crt if you install it w/ Visual C++/Studio)
it's worth understanding why this occured.
This patch
1. creates the pipe non-inherited, and dups the write end
for inheritence with limited permissions.
2. sets the STD_ERROR_HANDLE _after_ we fixup the FILE *stderr and fd 2.
3. Splits the signal_monitor event in two, service_init/service_term
for clarity and correctness.
Perform a serious scrubbing of the child process, before we report that
we failed to create the child. Cleans up CloseHandle() destruction to
match all created handles - and postpone populating the *child_exit_event
until we succeed.
This code seriously misassumed (and may yet) that stderr was a valid file.
It also has some bogus non-apr code that probably does great evil to win32
services. This must be bumped into .32, code review is incomplete still.
Greg Ames [Sun, 10 Feb 2002 21:16:25 +0000 (21:16 +0000)]
accept() error handling should be OK now.
I don't have a strong opinion on what we should do if the parent dies, now
that we don't have a real life example of that any more. It was a little
confusing at first, but OTOH it kept serving requests.
Greg Stein [Sat, 9 Feb 2002 01:57:38 +0000 (01:57 +0000)]
Get rid of the DAVParam support. That was a concept to help out
mod_dav providers in the 1.0.x series. Nowadays, the providers are
full-fledged Apache modules and can define their own directives and
set up per-server and per-directory parameters. (for example,
mod_dav_fs and the DAVLockDB directive)
Aaron Bannert [Thu, 7 Feb 2002 23:01:47 +0000 (23:01 +0000)]
Allow statically linked support binaries with the new
--enable-static-support flag, and enable this behavior in
the binbuild script. Also add a new --enable-static-htdbm
flag.
Allow mod_autoindex to serve symlinks if permitted and optimize our stats
so that only one stat() is needed.
If we get APR_FINFO_MIN bits, lookup_dirent won't do a repeated stat()
call. So, let's do it here. Also, if we see a symlink, expand it.
(Technically, we don't *have* to expand the symlinks - the resolve_symlink
fix handles that, but we can't really assume that.) Since we know that
dirent will be rr->finfo anyway, go back to relying on dirent only for
APR_DIR checks.
Fix resolve_symlink to save the original symlink name if known.
We would previously receive APR_INCOMPLETE on symlinks if wanted has
FINFO_NAME set because it isn't supported via apr_stat(). Furthermore, we
don't care what the real name is anyway (even if it apr_stat returned
.name) - we want to call it by the name the symlink says it is.
Eliminate tons o cruft - we are in another thread - and these can all
be longlived malloced regions, or sit upon the stack. No need to mess
with pool conflicts.
The remaining pool accesses are still a problem to resolve.
Upon advice of the FirstBill, I began looking for exceptions that occur
only in the service-mode. SetServiceDescription was borked, now fixed.
Since we don't use the posix/libc style stderr, I've also pitched that
code, which was causing an exception.
Cliff Woolley [Wed, 6 Feb 2002 22:32:34 +0000 (22:32 +0000)]
Changing my vote. I no longer have a problem with this. At the time,
lots of changes were being made to worker to fix bugs, and it seemed
premature. Haven't seen much of that as of late.
- We're not going to be doing an unmodified 2.0.31 release, so noting the
AP_DEBUG_ASSERT fix isn't helpful.
- The vote for perchild being required for GA is unanimous against. No
matter what interpretation you take, it isn't going to be a showstopper.
Ken Coar [Wed, 6 Feb 2002 19:42:12 +0000 (19:42 +0000)]
Separate showstoppers into next-beta and final-release sections.
Change a couple of my votes. Some minor formatting changes
(it's not code, so just deal with it ;-).
Move around a bit of code so I can follow it better. -> EOF should always
take us to the module struct, and working backwards, the register hooks
and then top-level init stuff. At least that's how any other Apache
module is authored.
Link to patch that incorporates all relevant bug fixes seen so far.
(only conflict from HEAD is in core.c where rbb/FirstBill snuck in some
hook changes...)
Give the child GENERIC_WRITE only to the stderr log channel,
GENERIC_READ|GENERIC_WRITE to the scoreboard mapping,
EVENT_MODIFY_STATE|SYNCHRONIZE to it's exit event,
and fix a _major_ bug in the NullACL code that set the length
by the size of the pointer-to-acl, rather than the size of an acl.
Ryan Bloom [Wed, 6 Feb 2002 16:58:37 +0000 (16:58 +0000)]
Be a bit more sane with regard to CanonicalNames. If the user has
specified they want to use the CanonicalName, but they have not
configured a port with the ServerName, then use the same port that
the original request used.
Jeff Trawick [Wed, 6 Feb 2002 16:19:03 +0000 (16:19 +0000)]
don't try to place a header field in error-notes if ap_rgetline()
returned something like APR_EOF, since len is not set in this
case and we can go off the end of storage
make sure we set r->status to something when we bail out due
to an error; otherwise, the request goes forward with meaningless
headers
Clean up more bogosity and leaky pipes [and fix a recent bug].
1. The only good assert is a deleted assert.
2. The child exit event is a very private item, can't pollute into
other processes we create, shouldn't be named, and should never
be accessable to anyone but the parent.
3. We now pass 'handles', not just a single scoreboard.
BS. This isn't an assert. This is a friggin developer's black hole.
Suggesion: Netware needs to implement the appropriate abort() style
fn. If there is none on Netware, this is non-portable, undebuggable,
and will be yanked by tommorow evening.
Start simplifying and grouping code. Building on the work that rbb
had begun; we need to start organizing this so it can be groked by
more than one person at a time [or simply by more than one person.]
We seem to have fixes in our tree for all of our cited problems.
If we could take the v31 tree and apply those revisions that fixes bugs
to make v32, I'd be game for beta.
Tone down the logging levels for these two messages from ERROR to NOTICE.
It's something to note, but it isn't an error worthy of logging by default.
(Also always log any status values in read_request_line() - incl. timeouts.)
- Fix up a comment so that it makes more sense and explains why we return
APR_EOF instead of an EOS bucket.
- Start to try to be nice when we *know* we are EOS by removing the
bucket. This is only one case where we could end up with a 0 bucket
in ctx->b, but let's remove it and move on. (If the bucket were to
return 0 bytes and still have data left to read on blocking mode,
it's blantly broken, IMHO)
While the tide was turning twords my take... this number of bugs in the
Winnt mpm mean I can't go against Unix getting a good beta. Now the
Q, is unix a good beta :-?
Jeff Trawick [Tue, 5 Feb 2002 22:56:44 +0000 (22:56 +0000)]
In core_input_filter, check for an empty brigade after
APR_BRIGADE_NORMALIZE(). Otherwise, we can get segfaults if a
client says it will post some data but we get FIN before any
data arrives.
Ryan Bloom [Tue, 5 Feb 2002 22:18:49 +0000 (22:18 +0000)]
Remove the install_transport_filters hook. The same function can be
acheived with the pre_connection hook. I have added the socket to the
pre_connection phase to make this possible.
Reviewed by: Bill Stoddard
projects / apache / log