Replaced MemObject::url with MemObject::urlXXX() and storeId().
* Replace StoreEntry::hidden_mem_obj hack with explicit MemObject::setUris().
We need MemObject to tie Store::get() results to locked memory cache entries
and such but Store::get() does not know the entry URIs so we had to use fake
"TBD" URIs instead. The hidden_mem_obj hack was added to minimize chances
that those temporary "TBD" URIs are going to be logged or forwarded.
However, new code uses MemObject cache ties a lot more, and it became too
cumbersome and error prone to always check whether there is a hidden object
holding indexes of locked StoreMap entries. It should be easier to ensure
that true URIs are set after Store::get() instead.
* Provide accessors for MemObject::url (which is actually a store ID these
days) and MemObject::log_url (which is usually the same as the url so we now
do not allocated it when it is the same). These accessors allow us to verify
that the caller is not going to use an undefined URI or Store ID because some
code forgot to set them explicitly.
* Add urlXXX() to mark old callers that appear to assume that MemObject::url
still holds a URI (instead of StoreID). Fixing those callers is outside this
project scope, but this was a good opportunity to identify/mark them because
we needed to hide raw Store ID field name ("url") anyway.
return Pool().inUseCount();
}
-void
-MemObject::resetUrls(char const *aUrl, char const *aLog_url)
-{
- safe_free(url);
- safe_free(log_url); /* XXX account log_url */
- log_url = xstrdup(aLog_url);
- url = xstrdup(aUrl);
+const char *
+MemObject::storeId() const {
+ if (!storeId_.defined()) {
+ debugs(20, DBG_IMPORTANT, "Bug: Missing MemObject::storeId value");
+ dump();
+ storeId_ = "[unknown_URI]";
+ }
+ return storeId_.termedBuf();
}
-MemObject::MemObject(char const *aUrl, char const *aLog_url):
- smpCollapsed(false)
-{
- debugs(20, 3, HERE << "new MemObject " << this);
- _reply = new HttpReply;
- HTTPMSGLOCK(_reply);
+const char *
+MemObject::logUri() const {
+ return logUri_.defined() ? logUri_.termedBuf() : storeId();
+}
- url = xstrdup(aUrl);
+bool
+MemObject::hasUris() const {
+ return storeId_.defined();
+}
-#if URL_CHECKSUM_DEBUG
+void
+MemObject::setUris(char const *aStoreId, char const *aLogUri, const HttpRequestMethod &aMethod)
+{
+ storeId_ = aStoreId;
+
+ // fast pointer comparison for a common storeCreateEntry(url,url,...) case
+ if (!aLogUri || aLogUri == aStoreId)
+ logUri_.clean(); // use storeId_ by default to minimize copying
+ else
+ logUri_ = aLogUri;
- chksum = url_checksum(url);
+ method = aMethod;
+#if URL_CHECKSUM_DEBUG
+ chksum = url_checksum(urlXXX());
#endif
+}
- log_url = xstrdup(aLog_url);
+MemObject::MemObject(): smpCollapsed(false)
+{
+ debugs(20, 3, HERE << "new MemObject " << this);
+ _reply = new HttpReply;
+ HTTPMSGLOCK(_reply);
object_sz = -1;
MemObject::~MemObject()
{
debugs(20, 3, HERE << "del MemObject " << this);
- const Ctx ctx = ctx_enter(url);
-#if URL_CHECKSUM_DEBUG
+ const Ctx ctx = ctx_enter(storeId_.termedBuf()); /* XXX: need URI? */
- assert(chksum == url_checksum(url));
+#if URL_CHECKSUM_DEBUG
+ checkUrlChecksum();
#endif
if (!shutting_down) { // Store::Root() is FATALly missing during shutdown
- // TODO: Consider moving these to destroyMemoryObject while providing
- // StoreEntry::memObjForDisconnect() or similar to get access to the
- // hidden memory object
+ // TODO: Consider moving these to destroyMemoryObject
if (xitTable.index >= 0)
Store::Root().transientsDisconnect(*this);
if (memCache.index >= 0)
ctx_exit(ctx); /* must exit before we free mem->url */
- safe_free(url);
-
- safe_free(log_url); /* XXX account log_url */
-
safe_free(vary_headers);
}
debugs(20, DBG_IMPORTANT, "MemObject->nclients: " << nclients);
debugs(20, DBG_IMPORTANT, "MemObject->reply: " << _reply);
debugs(20, DBG_IMPORTANT, "MemObject->request: " << request);
- debugs(20, DBG_IMPORTANT, "MemObject->log_url: " << checkNullString(log_url));
+ debugs(20, DBG_IMPORTANT, "MemObject->logUri: " << logUri_);
+ debugs(20, DBG_IMPORTANT, "MemObject->storeId: " << storeId_);
}
HttpReply const *
MemObject::stat(MemBuf * mb) const
{
mb->Printf("\t%s %s\n",
- RequestMethodStr(method), log_url);
+ RequestMethodStr(method), logUri());
if (vary_headers)
mb->Printf("\tvary_headers: %s\n", vary_headers);
mb->Printf("\tinmem_lo: %" PRId64 "\n", inmem_lo);
void
MemObject::checkUrlChecksum () const
{
- assert(chksum == url_checksum(url));
+ assert(chksum == url_checksum(urlXXX()));
}
#endif
MEMPROXY_CLASS(MemObject);
void dump() const;
- MemObject(char const *, char const *);
+ MemObject();
~MemObject();
- /// replaces construction-time URLs with correct ones; see hidden_mem_obj
- void resetUrls(char const *aUrl, char const *aLog_url);
+ /// sets store ID, log URI, and request method; TODO: find a better name
+ void setUris(char const *aStoreId, char const *aLogUri, const HttpRequestMethod &aMethod);
+
+ /// whether setUris() has been called
+ bool hasUris() const;
void write(const StoreIOBuffer &buf);
void unlinkRequest();
void checkUrlChecksum() const;
#endif
+ /// Before StoreID, code assumed that MemObject stores Request URI.
+ /// After StoreID, some old code still incorrectly assumes that.
+ /// Use this method to mark that incorrect assumption.
+ const char *urlXXX() const { return storeId(); }
+
+ /// Entry StoreID (usually just Request URI); if a buggy code requests this
+ /// before the information is available, returns an "[unknown_URI]" string.
+ const char *storeId() const;
+
+ /// client request URI used for logging; storeId() by default
+ const char *logUri() const;
+
HttpRequestMethod method;
- char *url;
mem_hdr data_hdr;
int64_t inmem_lo;
dlink_list clients;
STABH *callback;
void *data;
} abort;
- char *log_url;
RemovalPolicyNode repl;
int id;
int64_t object_sz;
private:
HttpReply *_reply;
+ mutable String storeId_; ///< StoreId for our entry (usually request URI)
+ mutable String logUri_; ///< URI used for logging (usually request URI)
+
DeferredReadManager deferredReads;
};
// XXX: We do not know the URLs yet, only the key, but we need to parse and
// store the response for the Root().get() callers to be happy because they
// expect IN_MEMORY entries to already have the response headers and body.
- // At least one caller calls createMemObject() if there is not one, so
- // we hide the true object until that happens (to avoid leaking TBD URLs).
- e->createMemObject("TBD", "TBD");
+ e->makeMemObject();
anchorEntry(*e, index, *slot);
e->mem_obj->memCache.index = -1;
e->mem_obj->memCache.io = MemObject::MemCache::ioDone;
- e->hideMemObject();
-
if (copied) {
e->hashInsert(key);
return e;
MemStore::updateCollapsed(StoreEntry &collapsed)
{
assert(collapsed.mem_status == IN_MEMORY);
- MemObject *mem_obj = collapsed.findMemObject();
- assert(mem_obj);
+ assert(collapsed.mem_obj);
- const sfileno index = mem_obj->memCache.index;
+ const sfileno index = collapsed.mem_obj->memCache.index;
// already disconnected from the cache, no need to update
if (index < 0)
MemStore::unlink(StoreEntry &e)
{
assert(e.mem_status == IN_MEMORY);
- MemObject *mem_obj = e.findMemObject();
- assert(mem_obj);
- if (mem_obj->memCache.index >= 0) {
- map->freeEntry(mem_obj->memCache.index);
- disconnect(*mem_obj);
+ assert(e.mem_obj);
+ if (e.mem_obj->memCache.index >= 0) {
+ map->freeEntry(e.mem_obj->memCache.index);
+ disconnect(*e.mem_obj);
} else {
// the entry was loaded and then disconnected from the memory cache
map->freeEntryByKey(reinterpret_cast<cache_key*>(e.key));
virtual const char *getMD5Text() const;
StoreEntry();
- StoreEntry(const char *url, const char *log_url);
virtual ~StoreEntry();
virtual HttpReply const *getReply() const;
int locked() const;
int validToSend() const;
bool memoryCachable() const; ///< may be cached in memory
- void createMemObject(const char *, const char *);
- void hideMemObject(); ///< no mem_obj for callers until createMemObject
- /// Memory cache needs this to get access to memCache.index of entries for
- /// which the caller did not call StoreEntry::createMemObject().
- MemObject *findMemObject() { return mem_obj ? mem_obj : hidden_mem_obj; }
+ /// if needed, initialize mem_obj member w/o URI-related information
+ MemObject *makeMemObject();
+
+ /// initialize mem_obj member (if needed) and supply URI-related info
+ void createMemObject(const char *storeId, const char *logUri, const HttpRequestMethod &aMethod);
void dump(int debug_lvl) const;
void hashDelete();
virtual RefCount<SwapDir> store() const;
MemObject *mem_obj;
- MemObject *hidden_mem_obj; ///< mem_obj created before URLs were known
RemovalPolicyNode repl;
/* START OF ON-DISK STORE_META_STD TLV field */
time_t timestamp;
{
assert (getType() == STORE_META_URL);
- if (!e->mem_obj->url)
+ if (!e->mem_obj->hasUris())
return true;
- if (strcasecmp(e->mem_obj->url, (char *)value)) {
+ if (strcasecmp(e->mem_obj->urlXXX(), (char *)value)) {
debugs(20, DBG_IMPORTANT, "storeClientReadHeader: URL mismatch");
- debugs(20, DBG_IMPORTANT, "\t{" << (char *) value << "} != {" << e->mem_obj->url << "}");
+ debugs(20, DBG_IMPORTANT, "\t{" << (char *) value << "} != {" << e->mem_obj->urlXXX() << "}");
return false;
}
if (vary) {
/* Oops... something odd is going on here.. */
debugs(33, DBG_IMPORTANT, "varyEvaluateMatch: Oops. Not a Vary object on second attempt, '" <<
- entry->mem_obj->url << "' '" << vary << "'");
+ entry->mem_obj->urlXXX() << "' '" << vary << "'");
safe_free(request->vary_headers);
return VARY_CANCEL;
}
* found the requested variant. Bail out
*/
debugs(33, DBG_IMPORTANT, "varyEvaluateMatch: Oops. Not a Vary match on second attempt, '" <<
- entry->mem_obj->url << "' '" << vary << "'");
+ entry->mem_obj->urlXXX() << "' '" << vary << "'");
return VARY_CANCEL;
}
}
*/
assert(http->logType == LOG_TCP_HIT);
- if (strcmp(e->mem_obj->url, http->request->storeId()) != 0) {
- debugs(33, DBG_IMPORTANT, "clientProcessHit: URL mismatch, '" << e->mem_obj->url << "' != '" << http->request->storeId() << "'");
+ if (strcmp(e->mem_obj->storeId(), http->request->storeId()) != 0) {
+ debugs(33, DBG_IMPORTANT, "clientProcessHit: URL mismatch, '" << e->mem_obj->storeId() << "' != '" << http->request->storeId() << "'");
processMiss();
return;
}
ErrorState *err = clientBuildError(ERR_ACCESS_DENIED, Http::scForbidden, NULL,
http->getConn()->clientConnection->remote, http->request);
startError(err);
- return; // XXX: leaking unused entry is some store does not keep it
+ return; // XXX: leaking unused entry if some store does not keep it
}
StoreIOBuffer localTempBuffer;
http->storeEntry(entry);
http->storeEntry()->lock();
- http->storeEntry()->createMemObject(storeId(), http->log_uri);
-
- http->storeEntry()->mem_obj->method = http->request->method;
+ http->storeEntry()->createMemObject(storeId(), http->log_uri,
+ http->request->method);
sc = storeClientListAdd(http->storeEntry(), this);
http->storeEntry()->lock();
- if (http->storeEntry()->mem_obj == NULL) {
+ MemObject *mem_obj = http->storeEntry()->makeMemObject();
+ if (!mem_obj->hasUris()) {
/*
* This if-block exists because we don't want to clobber
* a preexiting mem_obj->method value if the mem_obj
* is a cache hit for a GET response, we want to keep
* the method as GET.
*/
- http->storeEntry()->createMemObject(storeId(), http->log_uri);
- http->storeEntry()->mem_obj->method = http->request->method;
+ mem_obj->setUris(storeId(), http->log_uri, http->request->method);
/**
* Here we can see if the object was
* created using URL or alternative StoreID from helper.
*/
- debugs(88, 3, "mem_obj->url: " << http->storeEntry()->mem_obj->url);
+ debugs(88, 3, "storeId: " << http->storeEntry()->mem_obj->storeId());
}
sc = storeClientListAdd(http->storeEntry(), this);
if (e->mem_obj->request)
pe = storeGetPublicByRequest(e->mem_obj->request);
else
- pe = storeGetPublic(e->mem_obj->url, e->mem_obj->method);
+ pe = storeGetPublic(e->mem_obj->storeId(), e->mem_obj->method);
if (pe != NULL) {
assert(e != pe);
if (e->mem_obj->request)
pe = storeGetPublicByRequestMethod(e->mem_obj->request, Http::METHOD_HEAD);
else
- pe = storeGetPublic(e->mem_obj->url, Http::METHOD_HEAD);
+ pe = storeGetPublic(e->mem_obj->storeId(), Http::METHOD_HEAD);
if (pe != NULL) {
assert(e != pe);
* condition
*/
#define REFRESH_OVERRIDE(flag) \
- ((R = (R ? R : refreshLimits(entry->mem_obj->url))) , \
+ ((R = (R ? R : refreshLimits(entry->mem_obj->storeId()))) , \
(R && R->flags.flag))
#else
#define REFRESH_OVERRIDE(flag) 0
/** Creates a blank header. If this routine is made incremental, this will not do */
/* NP: all exit points to this function MUST call ctx_exit(ctx) */
- Ctx ctx = ctx_enter(entry->mem_obj->url);
+ Ctx ctx = ctx_enter(entry->mem_obj->urlXXX());
debugs(11, 3, "processReplyHeader: key '" << entry->getMD5Text() << "'");
{
ServerStateData::haveParsedReplyHeaders();
- Ctx ctx = ctx_enter(entry->mem_obj->url);
+ Ctx ctx = ctx_enter(entry->mem_obj->urlXXX());
HttpReply *rep = finalReply();
if (rep->sline.status() == Http::scPartialContent && rep->content_range)
debugs(72, 5, "peerDigestRequest: found old entry");
old_e->lock();
- old_e->createMemObject(url, url);
+ old_e->createMemObject(url, url, req->method);
fetch->old_sc = storeClientListAdd(old_e, fetch);
}
stale_flags sf;
if (entry->mem_obj)
- uri = entry->mem_obj->url;
+ uri = entry->mem_obj->storeId();
else if (request)
uri = urlCanonical(request);
StoreEntry::StoreEntry() :
mem_obj(NULL),
- hidden_mem_obj(NULL),
timestamp(-1),
lastref(-1),
expires(-1),
debugs(20, 3, HERE << "new StoreEntry " << this);
}
-StoreEntry::StoreEntry(const char *aUrl, const char *aLogUrl) :
- mem_obj(NULL),
- hidden_mem_obj(NULL),
- timestamp(-1),
- lastref(-1),
- expires(-1),
- lastmod(-1),
- swap_file_sz(0),
- refcount(0),
- flags(0),
- swap_filen(-1),
- swap_dirn(-1),
- lock_count(0),
- mem_status(NOT_IN_MEMORY),
- ping_status(PING_NONE),
- store_status(STORE_PENDING),
- swap_status(SWAPOUT_NONE)
-{
- debugs(20, 3, HERE << "new StoreEntry " << this);
- mem_obj = new MemObject(aUrl, aLogUrl);
-}
-
StoreEntry::~StoreEntry()
{
if (swap_filen >= 0) {
SwapDir &sd = dynamic_cast<SwapDir&>(*store());
sd.disconnect(*this);
}
- delete hidden_mem_obj;
}
#if USE_ADAPTATION
MemObject *mem = mem_obj;
mem_obj = NULL;
delete mem;
- delete hidden_mem_obj;
- hidden_mem_obj = NULL;
-}
-
-void
-StoreEntry::hideMemObject()
-{
- debugs(20, 3, HERE << "hiding " << mem_obj);
- assert(mem_obj);
- assert(!hidden_mem_obj);
- hidden_mem_obj = mem_obj;
- mem_obj = NULL;
}
void
hashDelete();
}
- if (mem_obj != NULL) {
+ if (mem_obj && mem_obj->hasUris()) {
mem_obj->id = getKeyCounter();
- newkey = storeKeyPrivate(mem_obj->url, mem_obj->method, mem_obj->id);
+ newkey = storeKeyPrivate(mem_obj->storeId(), mem_obj->method, mem_obj->id);
} else {
newkey = storeKeyPrivate("JUNK", Http::METHOD_NONE, getKeyCounter());
}
void
StoreEntry::setPublicKey()
{
- StoreEntry *e2 = NULL;
const cache_key *newkey;
if (key && !EBIT_TEST(flags, KEY_PRIVATE))
* to record the new variance key
*/
safe_free(request->vary_headers); /* free old "bad" variance key */
- StoreEntry *pe = storeGetPublic(mem_obj->url, mem_obj->method);
-
- if (pe)
+ if (StoreEntry *pe = storeGetPublic(mem_obj->storeId(), mem_obj->method))
pe->release();
}
// TODO: storeGetPublic() calls below may create unlocked entries.
// We should add/use storeHas() API or lock/unlock those entries.
- if (mem_obj->vary_headers && !storeGetPublic(mem_obj->url, mem_obj->method)) {
+ if (mem_obj->vary_headers && !storeGetPublic(mem_obj->storeId(), mem_obj->method)) {
/* Create "vary" base object */
String vary;
- StoreEntry *pe = storeCreateEntry(mem_obj->url, mem_obj->log_url, request->flags, request->method);
+ StoreEntry *pe = storeCreateEntry(mem_obj->storeId(), mem_obj->logUri(), request->flags, request->method);
/* We are allowed to do this typecast */
HttpReply *rep = new HttpReply;
rep->setHeaders(Http::scOkay, "Internal marker object", "x-squid-internal/vary", -1, -1, squid_curtime + 100000);
newkey = storeKeyPublicByRequest(mem_obj->request);
} else
- newkey = storeKeyPublic(mem_obj->url, mem_obj->method);
+ newkey = storeKeyPublic(mem_obj->storeId(), mem_obj->method);
- if ((e2 = (StoreEntry *) hash_lookup(store_table, newkey))) {
- debugs(20, 3, "StoreEntry::setPublicKey: Making old '" << mem_obj->url << "' private.");
+ if (StoreEntry *e2 = (StoreEntry *)hash_lookup(store_table, newkey)) {
+ debugs(20, 3, "Making old " << *e2 << " private.");
e2->setPrivateKey();
e2->release();
if (mem_obj->request)
newkey = storeKeyPublicByRequest(mem_obj->request);
else
- newkey = storeKeyPublic(mem_obj->url, mem_obj->method);
+ newkey = storeKeyPublic(mem_obj->storeId(), mem_obj->method);
}
if (key)
storeCreateEntry(const char *url, const char *log_url, const RequestFlags &flags, const HttpRequestMethod& method)
{
StoreEntry *e = NULL;
- MemObject *mem = NULL;
debugs(20, 3, "storeCreateEntry: '" << url << "'");
- e = new StoreEntry(url, log_url);
+ e = new StoreEntry();
e->lock_count = 1; /* Note lock here w/o calling storeLock() */
- mem = e->mem_obj;
- mem->method = method;
+ e->makeMemObject();
+ e->mem_obj->setUris(url, log_url, method);
if (neighbors_do_private_keys || !flags.hierarchical)
e->setPrivateKey();
assert(mem_obj->inmem_lo == 0);
if (EBIT_TEST(flags, ENTRY_SPECIAL)) {
- debugs(20, 4, "StoreEntry::setMemStatus: not inserting special " << mem_obj->url << " into policy");
+ debugs(20, 4, "not inserting special " << *this << " into policy");
} else {
mem_policy->Add(mem_policy, this, &mem_obj->repl);
- debugs(20, 4, "StoreEntry::setMemStatus: inserted mem node " << mem_obj->url << " key: " << getMD5Text());
+ debugs(20, 4, "inserted " << *this << " key: " << getMD5Text());
}
++hot_obj_count; // TODO: maintain for the shared hot cache as well
} else {
if (EBIT_TEST(flags, ENTRY_SPECIAL)) {
- debugs(20, 4, "StoreEntry::setMemStatus: special entry " << mem_obj->url);
+ debugs(20, 4, "not removing special " << *this << " from policy");
} else {
mem_policy->Remove(mem_policy, this, &mem_obj->repl);
- debugs(20, 4, "StoreEntry::setMemStatus: removed mem node " << mem_obj->url);
+ debugs(20, 4, "removed " << *this);
}
--hot_obj_count;
else if (mem_obj == NULL)
return "[null_mem_obj]";
else
- return mem_obj->url;
+ return mem_obj->storeId();
}
-void
-StoreEntry::createMemObject(const char *aUrl, const char *aLogUrl)
+MemObject *
+StoreEntry::makeMemObject()
{
- debugs(20, 3, "A mem_obj create attempted using : " << aUrl);
-
- if (mem_obj)
- return;
-
- if (hidden_mem_obj) {
- debugs(20, 3, HERE << "restoring " << hidden_mem_obj);
- mem_obj = hidden_mem_obj;
- hidden_mem_obj = NULL;
- mem_obj->resetUrls(aUrl, aLogUrl);
- return;
- }
+ if (!mem_obj)
+ mem_obj = new MemObject();
+ return mem_obj;
+}
- mem_obj = new MemObject(aUrl, aLogUrl);
+void
+StoreEntry::createMemObject(const char *aUrl, const char *aLogUrl, const HttpRequestMethod &aMethod)
+{
+ makeMemObject();
+ mem_obj->setUris(aUrl, aLogUrl, aMethod);
}
/* this just sets DELAY_SENDING */
if (sc->_callback.pending()) {
/* callback with ssize = -1 to indicate unexpected termination */
- debugs(90, 3, "storeUnregister: store_client for " << mem->url << " has a callback");
+ debugs(90, 3, "store_client for " << *e << " has a callback");
sc->fail();
}
++storeLogTagsCounts[tag];
if (mem != NULL) {
- if (mem->log_url == NULL) {
- debugs(20, DBG_IMPORTANT, "storeLog: NULL log_url for " << mem->url);
- mem->dump();
- mem->log_url = xstrdup(mem->url);
- }
-
reply = e->getReply();
/*
* XXX Ok, where should we print the dir number here?
reply->content_length,
e->contentLen(),
RequestMethodStr(mem->method),
- mem->log_url);
+ mem->logUri());
logfileLineEnd(storelog);
} else {
/* no mem object. Most RELEASE cases */
int StoreEntry::locked() const STUB_RETVAL(0)
int StoreEntry::validToSend() const STUB_RETVAL(0)
bool StoreEntry::memoryCachable() const STUB_RETVAL(false)
-void StoreEntry::createMemObject(const char *, const char *) STUB
-void StoreEntry::hideMemObject() STUB
+void StoreEntry::makeMemObject() STUB
+void StoreEntry::createMemObject(const char *, const char *, const HttpRequestMethod &aMethod) STUB
void StoreEntry::dump(int debug_lvl) const STUB
void StoreEntry::hashDelete() STUB
void StoreEntry::hashInsert(const cache_key *) STUB
projects / thirdparty / squid.git / commitdiff
