Alex Rousskov [Fri, 12 Sep 2008 03:57:47 +0000 (21:57 -0600)]
Aggregate commit after two --local commits:
- Cleaned up Comm: comm_close, comm_read_cancel, half-closed monitors, leaks.
- Cleaned up reconfiguration sequence.
Please see individual commit messages for details (bzr permitting).
Alex Rousskov [Fri, 12 Sep 2008 02:59:51 +0000 (20:59 -0600)]
Cleaned up reconfiguration sequence.
mainReconfigure() used to close and then open various sockets. Since
comm_close is now asynchronous, one cannot close and open in the same
function. Split mainReconfigure into mainReconfigureStart (that starts
the closing process for all relevant sockets) and mainReconfigureFinish
that opens the new sockets.
serverConnectionsClose is only used by main.cc and, hence, can be
static.
Polished comments and added an XXX comment on why SquidShutdown is
broken.
Also removed commCheckHalfClosed event scheduling. A separate cleanup
patch removes the associated half-closed monitoring loop.
Alex Rousskov [Fri, 12 Sep 2008 02:58:44 +0000 (20:58 -0600)]
Cleaned up Comm: comm_close, comm_read_cancel, half-closed monitors, leaks.
1) Comm_close now implements the following API:
Comm_close does not close the descriptor but initiates the following
closing sequence:
1) The descriptor is placed in a "closing" state.
2) The registered read, write, and accept callbacks (if any) are
scheduled (in an unspecified order).
3) The close callbacks are scheduled (in an unspecified order).
4) A call to the internal descriptor closing handler is
scheduled.
Details of the above steps are being documented separately and will
become a part of Comm API documentation.
Since all notifications are asynchronous, it is possible for a read or
write notification that was scheduled before comm_close was called to
arrive at its destination after comm_close was called. Such
notification will arrive with COMM_ERR_CLOSING flag even though that
flag was not set at the time of the I/O (and the I/O may have been
successful). CommIoCbParams::syncWithComm is used for this. The
credit for this trick goes to Christos Tsantilas.
Removed fde.flags.closing_ flag as unused.
2) Removed most of the half-closed monitoring code. Old code scheduled
monitoring reads every main loop iteration, I think. It is possible
that the assumption was that the handler will be activated and
cleared once per iteration so that the new read can be scheduled. The
design could result in conflicts between two monitoring reads and
possibly between a monitoring read and an active read. There were
also problems with handling closing descriptors.
I have removed the loop, AbortChecker, and the associated splay
tree). When user code marks the descriptor as half-closed, Comm now
simply schedules a monitoring read callback. If the user needs to
check whether the descriptor was marked, Comm checks whether the
callback is present. If a user schedules a read when there is
already a monitoring callback, the monitoring callback is removed.
Renamed user-facing monitoring functions but left compatibility
wrappers in place to minimize user code changes, for now.
It is possible that the whole half-closed monitoring code will be
eventually deleted. The above changes are meant to preserve the
intended functionality (but without coredumps) while the decision is
being made.
3) Removed _SQUID_LINUX_-only code that would avoid addrinfo destruction
on connect "errors". Squid seems to be working fine without this
code. With this code, we leak memory on many connect requests because
of EINPROGRESS. More work is probably needed to reproduce and fix the
true cause of the memory corruption observed earlier. Removing the
workaround will allow us to get more bug reports if the problem is
still there.
Alex Rousskov [Fri, 12 Sep 2008 02:52:50 +0000 (20:52 -0600)]
Cleaned up ConnStateData's closing and destruction.
1) Despite its name and the "if (open) close" use in ConnStateData
destructor, ConnStateData::close() was not closing anything. It was
called from the Comm close handler and from the destructor and would
attempt to immediately delete the ConnStateData object. Protecting code
in deleteThis() may have prevented the actual [double] delete from
happening, but it is difficult to say exactly what was going on when the
close() method was being called from the destructor.
I converted ConnStateData::close to swanSong, which is the standard
AsyncJob cleanup method. As before, the method does not close anything
(which may still be wrong). The swanSong method is never called directly
by the user code. It is called by lower layers just before the job is
destroyed. The updated close handler initiates job destruction by
calling deleteThis().
We may need to add Comm closing code to swanSong. For now, the updated
ConnStateData destructor will warn if ConnStateData forgot to close the
connection. The destructor will also warn if swanSong was not called,
which would mean that the job object is being deleted incorrectly.
2) Polished ClientSocketContext::writeComplete to distinguish
STREAM_UNPLANNED_COMPLETE from STREAM_FAILED closing state. This helps
when looking at stack traces.
3) Added an XXX comment about duplicated code.
4) Documented ClientSocketContext::initiateClose purpose and context.
Bug 1628: follow_x_forwarded_for shoudl not cause allow/deny behavior
clientFollowXForwardedForCheck() needs to always set the
request->indirect_client_addr properly at completion and call
calloutContext->clientAccessCheck(); unconditionally to begin actual
access ACL tests.
Calling clientAccessCheckDone(answer) is equivalent to processing an
http_access line with denial.
Alex Rousskov [Thu, 11 Sep 2008 06:32:57 +0000 (00:32 -0600)]
Cleaned up reconfiguration sequence.
mainReconfigure() used to close and then open various sockets. Since
comm_close is now asynchronous, one cannot close and open in the same
function. Split mainReconfigure into mainReconfigureStart (that starts
the closing process for all relevant sockets) and mainReconfigureFinish
that opens the new sockets.
serverConnectionsClose is only used by main.cc and, hence, can be static.
Polished comments and added an XXX comment on why SquidShutdown is broken.
Also removed commCheckHalfClosed event scheduling. A separate cleanup patch
removes the associated half-closed monitoring loop.
Alex Rousskov [Thu, 11 Sep 2008 05:58:32 +0000 (23:58 -0600)]
Cleaned up Comm: comm_close, comm_read_cancel, half-closed monitors,
leaks.
1) Comm_close now implements the following API:
Comm_close does not close the descriptor but initiates the following
closing sequence:
1) The descriptor is placed in a "closing" state.
2) The registered read, write, and accept callbacks (if any) are
scheduled (in an unspecified order).
3) The close callbacks are scheduled (in an unspecified order).
4) A call to the internal descriptor closing handler is
scheduled.
Details of the above steps are being documented separately and will
become a part of Comm API documentation.
Since all notifications are asynchronous, it is possible for a read or
write notification that was scheduled before comm_close was called to
arrive at its destination after comm_close was called. Such
notification will arrive with COMM_ERR_CLOSING flag even though that
flag was not set at the time of the I/O (and the I/O may have been
successful). CommIoCbParams::syncWithComm is used for this. The
credit for this trick goes to Christos Tsantilas.
Removed fde.flags.closing_ flag as unused.
2) Removed most of the half-closed monitoring code. Old code scheduled
monitoring reads every main loop iteration, I think. It is possible
that the assumption was that the handler will be activated and
cleared once per iteration so that the new read can be scheduled. The
design could result in conflicts between two monitoring reads and
possibly between a monitoring read and an active read. There were
also problems with handling closing descriptors.
I have removed the loop, AbortChecker, and the associated splay
tree). When user code marks the descriptor as half-closed, Comm now
simply schedules a monitoring read callback. If the user needs to
check whether the descriptor was marked, Comm checks whether the
callback is present. If a user schedules a read when there is
already a monitoring callback, the monitoring callback is removed.
Renamed user-facing monitoring functions but left compatibility
wrappers in place to minimize user code changes, for now.
It is possible that the whole half-closed monitoring code will be
eventually deleted. The above changes are meant to preserve the
intended functionality (but without coredumps) while the decision is
being made.
3) Removed _SQUID_LINUX_-only code that would avoid addrinfo destruction
on connect "errors". Squid seems to be working fine without this
code. With this code, we leak memory on many connect requests because
of EINPROGRESS. More work is probably needed to reproduce and fix the
true cause of the memory corruption observed earlier. Removing the
workaround will allow us to get more bug reports if the problem is
still there.
My braindead alteration to pass the base data and config directories to
the code for use at compile-time backfired with ./configure adding variable
names intended for automake into the autoconf.h file for build.
This approach drops any fancy definition/substitution attempts
and simply adds an compiler flag parameter to every object build.
Alex Rousskov [Thu, 11 Sep 2008 04:54:34 +0000 (22:54 -0600)]
Cleaned up ConnStateData's closing and destruction.
1) Despite its name and the "if (open) close" use in ConnStateData destructor,
ConnStateData::close() was not closing anything. It was called from the Comm
close handler and from the destructor and would attempt to immediately delete
the ConnStateData object. Protecting code in deleteThis() may have prevented
the actual [double] delete from happening, but it is difficult to say exactly
what was going on when close() was being called from the destructor.
I converted ConnStateData::close to swanSong, which is the standard AsyncJob
cleanup method. As before, the method does not close anything (which may be
wrong). The swanSong method is never called directly by the user code. It is
called by lower layers just before the job is destroyed.
We may need to add Comm closing code to swanSong. For now, the updated
ConnStateData destructor will warn if ConnStateData forgot to close
the connection. The destructor will also warn if swanSong was not called,
which would mean that the job object is being deleted incorrectly.
2) Polished ClientSocketContext::writeComplete to distinguish
STREAM_UNPLANNED_COMPLETE from STREAM_FAILED closing state. This helps when
looking at stack traces.
3) Added an XXX comment about duplicated code.
4) Documented ClientSocketContext::initiateClose purpose and context.
Alex Rousskov [Sat, 6 Sep 2008 05:15:20 +0000 (23:15 -0600)]
Cleanup fde data members before memset in fde::clean() corrupts their state.
This prevents fde::timeoutHandler and fde::closeHandler memory leaks but there
will probably be more corruption as fde data members are added or changed.
Made fde::clean() private to prevent it from spreading through the code.
Use a new fde object to clean an old one, for now.
- Change default invalidation behaviour to match recommended behaviour in RFC.
- Ensure URLs from Location and Content-Location headers are absolute before
trying to find their associated objects in the store.
Author: Marin Stavrev <mstavrev@gmail.com>
Fix bug in ZPH parent/sibling hit marking.
When a peer (parent or sibling) hit was detected the TOS was changed using
the local hit TOS marking (zph_tos_local) value instead of the one
configured in the zph_tos_peer parameter.
- Use bool instead of int for urlIsRelative.
- Document what leads to a NULL return in urlMakeAbsolute.
- Declare variables closer to where they're used.
- Fix indentation.
- Split urlAbsolute into urlIsRelative and urlMakeAbsolute.
- Make urlIsRelative compliant with the RFC as to what defines a relative URL.
- Rework purgeEntriesByHeader to be a little easier on the eyes.
Rework urlAbsolute to be a little more streamlined.
The primary aim of this is to cut down on the number of ways snprintf was
called in the original version. The idea here is to build the common base
portion of the url using snprintf and then construct the rest using str[n]cpy.
snprintf is still used as the alternative (using only POSIX routines) involves
a much longer run of code that, at least in my estimation, gains us very little
over snprintf.
- Switch the default from not purging if the method is unknown to purging if
the method is unknown.
- When purging URIs sourced from Location and Content-Location headers, make
sure the URL is absolute before a) comparing it to see if hosts match and b)
actually trying to find it in the store.
Amos Jeffries [Wed, 27 Aug 2008 13:01:36 +0000 (01:01 +1200)]
Author: Benno Rice <benno@squid-cache.org>
Respect $(MAKE) in error translation build.
On FreeBSD, make is not GNU make. GNU make can be installed from ports, but it
is installed as gmake, not make. This makes it vital that Makefiles that wish
to work on FreeBSD always invoke make with $(MAKE) instead of make. The recent
error translation stuff did not follow this, and thus the build broke on
FreeBSD.
Amos Jeffries [Mon, 25 Aug 2008 13:40:16 +0000 (01:40 +1200)]
Author: Francesco Chemolli <kinkie@squid-cache.org>
TestBed: Fix layer-01 permutation options
This patch addresses three related issues:
- makes the error messages for those cases more informative
- changes some (echo + exit) sequences into AC_MSG_ERROR() standard autoconf macros
- changes the layer-01 test options so that it doesn't invoke invalid configure options
Henrik Nordstrom [Wed, 13 Aug 2008 03:19:05 +0000 (05:19 +0200)]
GCC 4.3 changed semantics of "extern inline" to that of C99 (same as inline),
but we assume GNU GCC semantics. This sets the needed attribute to tell GCC
to continue using GNU GCC semantics on this function.
Amos Jeffries [Tue, 12 Aug 2008 12:59:50 +0000 (00:59 +1200)]
Polish translation mechanism
Adds an automatic test for po2html binary and only attempts translation
if one is located.
The po2html path can be specified using: --with-po2html=/path/po2html
Amos Jeffries [Tue, 12 Aug 2008 12:12:37 +0000 (00:12 +1200)]
Author: Francesco Salvestrini and Dustin J. Mitchell
Extend configure with ax_with_prog
Locates an installed program binary, placing the result in the precious
variable VARIABLE. Accepts a present VARIABLE, then --with-program, and
failing that searches for program in the given path (which defaults to
the system path). If program is found, VARIABLE is set to the full path
of the binary; if it is not found VARIABLE is set to VALUE-IF-NOT-FOUND
if provided, unchanged otherwise.
NOTE: This macro is based upon the original AX_WITH_PYTHON macro from
Dustin J. Mitchell <dustin@cs.uchicago.edu>.
COPYLEFT
Copyright (c) 2008 Francesco Salvestrini <salvestrini@users.sourceforge.net>
Copyright (c) 2008 Dustin J. Mitchell <dustin@cs.uchicago.edu>
Amos Jeffries [Sun, 10 Aug 2008 05:49:14 +0000 (23:49 -0600)]
Fix: cppunit tests broken by squid.h defines
In order to promote safe coding and the use of internal accounting API
Squid mangles certain common function definitions such as malloc/calloc.
While this is a great idea for Squid internal code. It's not good when
integrating external cppunit macro libraries. At least one of which in
use performs its own allocation during testing.
This adds an extra layer of protection to prevent redirection with certain
unit-tests where the macros needed clash with Squid.
projects / thirdparty / squid.git / log
