From e0acd602e0c4f3af87ba34cbad85ca85594705f5 Mon Sep 17 00:00:00 2001 From: Shengliang Guan Date: Wed, 1 Jul 2020 13:31:26 +0800 Subject: [PATCH] [TD-814] invalid read while close http connect --- src/plugins/http/src/httpContext.c | 9 +++++++-- src/plugins/http/src/httpHandle.c | 24 +++++++++++------------ src/plugins/http/src/httpJson.c | 16 +++++++-------- src/plugins/http/src/httpServer.c | 2 -- src/plugins/http/src/httpSystem.c | 6 ++++-- src/util/src/tcache.c | 31 ++++++++++++++++-------------- 6 files changed, 48 insertions(+), 40 deletions(-) diff --git a/src/plugins/http/src/httpContext.c b/src/plugins/http/src/httpContext.c index b078be5930..b09f34b562 100644 --- a/src/plugins/http/src/httpContext.c +++ b/src/plugins/http/src/httpContext.c @@ -141,10 +141,15 @@ HttpContext *httpGetContext(void *ptr) { void httpReleaseContext(HttpContext *pContext) { int32_t refCount = atomic_sub_fetch_32(&pContext->refCount, 1); assert(refCount >= 0); - httpDebug("context:%p, fd:%d, is releasd, refCount:%d", pContext, pContext->fd, refCount); + httpDebug("context:%p, is releasd, refCount:%d", pContext, refCount); HttpContext **ppContext = pContext->ppContext; - taosCacheRelease(tsHttpServer.contextCache, (void **)(&ppContext), false); + if (tsHttpServer.contextCache != NULL) { + taosCacheRelease(tsHttpServer.contextCache, (void **)(&ppContext), false); + } else { + httpDebug("context:%p, won't be destroyed for cache is already released", pContext); + // httpDestroyContext((void **)(&ppContext)); + } } bool httpInitContext(HttpContext *pContext) { diff --git a/src/plugins/http/src/httpHandle.c b/src/plugins/http/src/httpHandle.c index 758286344a..056fe425d4 100644 --- a/src/plugins/http/src/httpHandle.c +++ b/src/plugins/http/src/httpHandle.c @@ -157,7 +157,7 @@ bool httpGetHttpMethod(HttpContext* pContext) { pParser->method.pos[pParser->method.len] = 0; pParser->pLast = pSeek + 1; - httpDebug("context:%p, fd:%d, ip:%s, httpMethod:%s", pContext, pContext->fd, pContext->ipstr, pParser->method.pos); + httpTrace("context:%p, fd:%d, ip:%s, httpMethod:%s", pContext, pContext->fd, pContext->ipstr, pParser->method.pos); return true; } @@ -186,23 +186,23 @@ bool httpParseHead(HttpContext* pContext) { HttpParser* pParser = &pContext->parser; if (strncasecmp(pParser->pLast, "Content-Length: ", 16) == 0) { pParser->data.len = (int32_t)atoi(pParser->pLast + 16); - httpDebug("context:%p, fd:%d, ip:%s, Content-Length:%d", pContext, pContext->fd, pContext->ipstr, + httpTrace("context:%p, fd:%d, ip:%s, Content-Length:%d", pContext, pContext->fd, pContext->ipstr, pParser->data.len); } else if (strncasecmp(pParser->pLast, "Accept-Encoding: ", 17) == 0) { if (tsHttpEnableCompress && strstr(pParser->pLast + 17, "gzip") != NULL) { pContext->acceptEncoding = HTTP_COMPRESS_GZIP; - httpDebug("context:%p, fd:%d, ip:%s, Accept-Encoding:gzip", pContext, pContext->fd, pContext->ipstr); + httpTrace("context:%p, fd:%d, ip:%s, Accept-Encoding:gzip", pContext, pContext->fd, pContext->ipstr); } else { pContext->acceptEncoding = HTTP_COMPRESS_IDENTITY; - httpDebug("context:%p, fd:%d, ip:%s, Accept-Encoding:identity", pContext, pContext->fd, pContext->ipstr); + httpTrace("context:%p, fd:%d, ip:%s, Accept-Encoding:identity", pContext, pContext->fd, pContext->ipstr); } } else if (strncasecmp(pParser->pLast, "Content-Encoding: ", 18) == 0) { if (strstr(pParser->pLast + 18, "gzip") != NULL) { pContext->contentEncoding = HTTP_COMPRESS_GZIP; - httpDebug("context:%p, fd:%d, ip:%s, Content-Encoding:gzip", pContext, pContext->fd, pContext->ipstr); + httpTrace("context:%p, fd:%d, ip:%s, Content-Encoding:gzip", pContext, pContext->fd, pContext->ipstr); } else { pContext->contentEncoding = HTTP_COMPRESS_IDENTITY; - httpDebug("context:%p, fd:%d, ip:%s, Content-Encoding:identity", pContext, pContext->fd, pContext->ipstr); + httpTrace("context:%p, fd:%d, ip:%s, Content-Encoding:identity", pContext, pContext->fd, pContext->ipstr); } } else if (strncasecmp(pParser->pLast, "Connection: ", 12) == 0) { if (strncasecmp(pParser->pLast + 12, "Keep-Alive", 10) == 0) { @@ -210,7 +210,7 @@ bool httpParseHead(HttpContext* pContext) { } else { pContext->httpKeepAlive = HTTP_KEEPALIVE_DISABLE; } - httpDebug("context:%p, fd:%d, ip:%s, keepAlive:%d", pContext, pContext->fd, pContext->ipstr, + httpTrace("context:%p, fd:%d, ip:%s, keepAlive:%d", pContext, pContext->fd, pContext->ipstr, pContext->httpKeepAlive); } else if (strncasecmp(pParser->pLast, "Transfer-Encoding: ", 19) == 0) { if (strncasecmp(pParser->pLast + 19, "chunked", 7) == 0) { @@ -281,7 +281,7 @@ bool httpReadChunkedBody(HttpContext* pContext, HttpParser* pParser) { httpParseChunkedBody(pContext, pParser, false); return HTTP_CHECK_BODY_SUCCESS; } else { - httpDebug("context:%p, fd:%d, ip:%s, chunked body not finished, continue read", pContext, pContext->fd, pContext->ipstr); + httpTrace("context:%p, fd:%d, ip:%s, chunked body not finished, continue read", pContext, pContext->fd, pContext->ipstr); if (!httpReadDataImp(pContext)) { httpError("context:%p, fd:%d, ip:%s, read chunked request error", pContext, pContext->fd, pContext->ipstr); return HTTP_CHECK_BODY_ERROR; @@ -299,7 +299,7 @@ int httpReadUnChunkedBody(HttpContext* pContext, HttpParser* pParser) { httpSendErrorResp(pContext, HTTP_PARSE_BODY_ERROR); return HTTP_CHECK_BODY_ERROR; } else if (dataReadLen < pParser->data.len) { - httpDebug("context:%p, fd:%d, ip:%s, un-chunked body not finished, read size:%d dataReadLen:%d < pContext->data.len:%d, continue read", + httpTrace("context:%p, fd:%d, ip:%s, un-chunked body not finished, read size:%d dataReadLen:%d < pContext->data.len:%d, continue read", pContext, pContext->fd, pContext->ipstr, pContext->parser.bufsize, dataReadLen, pParser->data.len); return HTTP_CHECK_BODY_CONTINUE; } else { @@ -313,9 +313,9 @@ bool httpParseRequest(HttpContext* pContext) { return true; } - httpDebug("context:%p, fd:%d, ip:%s, thread:%s, numOfFds:%d, read size:%d, raw data:\n%s", - pContext, pContext->fd, pContext->ipstr, pContext->pThread->label, pContext->pThread->numOfFds, - pContext->parser.bufsize, pContext->parser.buffer); + httpTraceDump("context:%p, fd:%d, ip:%s, thread:%s, numOfFds:%d, read size:%d, raw data:\n%s", pContext, pContext->fd, + pContext->ipstr, pContext->pThread->label, pContext->pThread->numOfFds, pContext->parser.bufsize, + pContext->parser.buffer); if (!httpGetHttpMethod(pContext)) { return false; diff --git a/src/plugins/http/src/httpJson.c b/src/plugins/http/src/httpJson.c index 319d87d496..9276637d0e 100644 --- a/src/plugins/http/src/httpJson.c +++ b/src/plugins/http/src/httpJson.c @@ -76,8 +76,8 @@ int httpWriteBuf(struct HttpContext *pContext, const char *buf, int sz) { httpError("context:%p, fd:%d, ip:%s, dataSize:%d, writeSize:%d, failed to send response:\n%s", pContext, pContext->fd, pContext->ipstr, sz, writeSz, buf); } else { - httpDebug("context:%p, fd:%d, ip:%s, dataSize:%d, writeSize:%d, response:\n%s", - pContext, pContext->fd, pContext->ipstr, sz, writeSz, buf); + httpTrace("context:%p, fd:%d, ip:%s, dataSize:%d, writeSize:%d, response:\n%s", pContext, pContext->fd, + pContext->ipstr, sz, writeSz, buf); } return writeSz; @@ -99,7 +99,7 @@ int httpWriteJsonBufBody(JsonBuf* buf, bool isTheLast) { uint64_t srcLen = (uint64_t) (buf->lst - buf->buf); if (buf->pContext->fd <= 0) { - httpDebug("context:%p, fd:%d, ip:%s, write json body error", buf->pContext, buf->pContext->fd, buf->pContext->ipstr); + httpTrace("context:%p, fd:%d, ip:%s, write json body error", buf->pContext, buf->pContext->fd, buf->pContext->ipstr); buf->pContext->fd = -1; } @@ -113,11 +113,11 @@ int httpWriteJsonBufBody(JsonBuf* buf, bool isTheLast) { if (buf->pContext->acceptEncoding == HTTP_COMPRESS_IDENTITY) { if (buf->lst == buf->buf) { - httpDebug("context:%p, fd:%d, ip:%s, no data need dump", buf->pContext, buf->pContext->fd, buf->pContext->ipstr); + httpTrace("context:%p, fd:%d, ip:%s, no data need dump", buf->pContext, buf->pContext->fd, buf->pContext->ipstr); return 0; // there is no data to dump. } else { int len = sprintf(sLen, "%lx\r\n", srcLen); - httpDebug("context:%p, fd:%d, ip:%s, write body, chunkSize:%" PRIu64 ", response:\n%s", + httpTrace("context:%p, fd:%d, ip:%s, write body, chunkSize:%" PRIu64 ", response:\n%s", buf->pContext, buf->pContext->fd, buf->pContext->ipstr, srcLen, buf->buf); httpWriteBufNoTrace(buf->pContext, sLen, len); remain = httpWriteBufNoTrace(buf->pContext, buf->buf, (int) srcLen); @@ -129,12 +129,12 @@ int httpWriteJsonBufBody(JsonBuf* buf, bool isTheLast) { if (ret == 0) { if (compressBufLen > 0) { int len = sprintf(sLen, "%x\r\n", compressBufLen); - httpDebug("context:%p, fd:%d, ip:%s, write body, chunkSize:%" PRIu64 ", compressSize:%d, last:%d, response:\n%s", + httpTrace("context:%p, fd:%d, ip:%s, write body, chunkSize:%" PRIu64 ", compressSize:%d, last:%d, response:\n%s", buf->pContext, buf->pContext->fd, buf->pContext->ipstr, srcLen, compressBufLen, isTheLast, buf->buf); httpWriteBufNoTrace(buf->pContext, sLen, len); remain = httpWriteBufNoTrace(buf->pContext, (const char *) compressBuf, (int) compressBufLen); } else { - httpDebug("context:%p, fd:%d, ip:%s, last:%d, compress already dumped, response:\n%s", + httpTrace("context:%p, fd:%d, ip:%s, last:%d, compress already dumped, response:\n%s", buf->pContext, buf->pContext->fd, buf->pContext->ipstr, isTheLast, buf->buf); return 0; // there is no data to dump. } @@ -173,7 +173,7 @@ void httpWriteJsonBufHead(JsonBuf* buf) { void httpWriteJsonBufEnd(JsonBuf* buf) { if (buf->pContext->fd <= 0) { - httpDebug("context:%p, fd:%d, ip:%s, json buf fd is 0", buf->pContext, buf->pContext->fd, buf->pContext->ipstr); + httpTrace("context:%p, fd:%d, ip:%s, json buf fd is 0", buf->pContext, buf->pContext->fd, buf->pContext->ipstr); buf->pContext->fd = -1; } diff --git a/src/plugins/http/src/httpServer.c b/src/plugins/http/src/httpServer.c index cef0e80690..6c82386d81 100644 --- a/src/plugins/http/src/httpServer.c +++ b/src/plugins/http/src/httpServer.c @@ -66,8 +66,6 @@ void httpCleanUpConnect() { } } - tfree(pServer->pThreads); - pServer->pThreads = NULL; httpDebug("http server:%s is cleaned up", pServer->label); } diff --git a/src/plugins/http/src/httpSystem.c b/src/plugins/http/src/httpSystem.c index a93e7cd7ad..3a0998f2e8 100644 --- a/src/plugins/http/src/httpSystem.c +++ b/src/plugins/http/src/httpSystem.c @@ -95,11 +95,13 @@ void httpCleanUpSystem() { httpInfo("http server cleanup"); httpStopSystem(); + httpCleanUpConnect(); httpCleanupContexts(); httpCleanUpSessions(); - httpCleanUpConnect(); pthread_mutex_destroy(&tsHttpServer.serverMutex); - + tfree(tsHttpServer.pThreads); + tsHttpServer.pThreads = NULL; + tsHttpServer.status = HTTP_SERVER_CLOSED; } diff --git a/src/util/src/tcache.c b/src/util/src/tcache.c index 2e57ad83ae..720741f089 100644 --- a/src/util/src/tcache.c +++ b/src/util/src/tcache.c @@ -119,7 +119,7 @@ static FORCE_INLINE void taosCacheReleaseNode(SCacheObj *pCacheObj, SCacheDataNo int32_t size = pNode->size; taosHashRemove(pCacheObj->pHashTable, pNode->key, pNode->keySize); - uDebug("key:%s is removed from cache,total:%" PRId64 ",size:%dbytes", pNode->key, pCacheObj->totalSize, size); + uDebug("key:%s, is removed from cache, total:%" PRId64 " size:%d bytes", pNode->key, pCacheObj->totalSize, size); if (pCacheObj->freeFp) pCacheObj->freeFp(pNode->data); free(pNode); } @@ -288,14 +288,14 @@ void *taosCachePut(SCacheObj *pCacheObj, const char *key, const void *pData, siz if (NULL != pNode) { pCacheObj->totalSize += pNode->size; - uDebug("key:%s %p added into cache, added:%" PRIu64 ", expire:%" PRIu64 ", total:%" PRId64 ", size:%" PRId64 " bytes", + uDebug("key:%s, %p added into cache, added:%" PRIu64 ", expire:%" PRIu64 ", total:%" PRId64 ", size:%" PRId64 " bytes", key, pNode, pNode->addedTime, pNode->expiredTime, pCacheObj->totalSize, dataSize); } else { - uError("key:%s failed to added into cache, out of memory", key); + uError("key:%s, failed to added into cache, out of memory", key); } } else { // old data exists, update the node pNode = taosUpdateCacheImpl(pCacheObj, pOld, key, keyLen, pData, dataSize, duration * 1000L); - uDebug("key:%s %p exist in cache, updated", key, pNode); + uDebug("key:%s, %p exist in cache, updated", key, pNode); } __cache_unlock(pCacheObj); @@ -321,10 +321,10 @@ void *taosCacheAcquireByName(SCacheObj *pCacheObj, const char *key) { if (ptNode != NULL) { atomic_add_fetch_32(&pCacheObj->statistics.hitCount, 1); - uDebug("key:%s is retrieved from cache, %p refcnt:%d", key, (*ptNode), T_REF_VAL_GET(*ptNode)); + uDebug("key:%s, is retrieved from cache, %p refcnt:%d", key, (*ptNode), T_REF_VAL_GET(*ptNode)); } else { atomic_add_fetch_32(&pCacheObj->statistics.missCount, 1); - uDebug("key:%s not in cache, retrieved failed", key); + uDebug("key:%s, not in cache, retrieved failed", key); } atomic_add_fetch_32(&pCacheObj->statistics.totalAccess, 1); @@ -350,10 +350,10 @@ void* taosCacheUpdateExpireTimeByName(SCacheObj *pCacheObj, const char *key, uin if (ptNode != NULL) { atomic_add_fetch_32(&pCacheObj->statistics.hitCount, 1); - uDebug("key:%s expireTime is updated in cache, %p refcnt:%d", key, (*ptNode), T_REF_VAL_GET(*ptNode)); + uDebug("key:%s, expireTime is updated in cache, %p refcnt:%d", key, (*ptNode), T_REF_VAL_GET(*ptNode)); } else { atomic_add_fetch_32(&pCacheObj->statistics.missCount, 1); - uDebug("key:%s not in cache, retrieved failed", key); + uDebug("key:%s, not in cache, retrieved failed", key); } atomic_add_fetch_32(&pCacheObj->statistics.totalAccess, 1); @@ -410,13 +410,13 @@ void taosCacheRelease(SCacheObj *pCacheObj, void **data, bool _remove) { SCacheDataNode *pNode = (SCacheDataNode *)((char *)(*data) - offset); if (pNode->signature != (uint64_t)pNode) { - uError("key: %p release invalid cache data", pNode); + uError("%p release invalid cache data", pNode); return; } *data = NULL; int32_t ref = T_REF_DEC(pNode); - uDebug("%p data released, refcnt:%d", pNode, ref); + uDebug("key:%s, is released, %p refcnt:%d", pNode->key, pNode, ref); if (_remove) { __cache_wr_lock(pCacheObj); @@ -501,7 +501,7 @@ void taosAddToTrash(SCacheObj *pCacheObj, SCacheDataNode *pNode) { pNode->inTrashCan = true; pCacheObj->numOfElemsInTrash++; - uDebug("key:%s %p move to trash, numOfElem in trash:%d", pNode->key, pNode, pCacheObj->numOfElemsInTrash); + uDebug("key:%s, %p move to trash, numOfElem in trash:%d", pNode->key, pNode, pCacheObj->numOfElemsInTrash); } void taosRemoveFromTrashCan(SCacheObj *pCacheObj, STrashElem *pElem) { @@ -549,7 +549,7 @@ void taosTrashCanEmpty(SCacheObj *pCacheObj, bool force) { } if (force || (T_REF_VAL_GET(pElem->pData) == 0)) { - uDebug("key:%s %p removed from trash. numOfElem in trash:%d", pElem->pData->key, pElem->pData, + uDebug("key:%s, %p removed from trash. numOfElem in trash:%d", pElem->pData->key, pElem->pData, pCacheObj->numOfElemsInTrash - 1); STrashElem *p = pElem; @@ -570,8 +570,11 @@ void doCleanupDataCache(SCacheObj *pCacheObj) { while (taosHashIterNext(pIter)) { SCacheDataNode *pNode = *(SCacheDataNode **)taosHashIterGet(pIter); // if (pNode->expiredTime <= expiredTime && T_REF_VAL_GET(pNode) <= 0) { - taosCacheReleaseNode(pCacheObj, pNode); - //} + if (T_REF_VAL_GET(pNode) <= 0) { + taosCacheReleaseNode(pCacheObj, pNode); + } else { + uDebug("key:%s, will not remove from cache, refcnt:%d", pNode->key, T_REF_VAL_GET(pNode)); + } } taosHashDestroyIter(pIter); -- GitLab