# HG changeset patch # User Sebastien Jodogne # Date 1589959175 -7200 # Node ID 620e87e9e8162717cddbc1ff084a413feaae4dcf # Parent 4f09a6a2b369214e3fc76db0757e78b58274508c c-get: fixing memory with failedUIDs_ diff -r 4f09a6a2b369 -r 620e87e9e816 Core/DicomNetworking/IGetRequestHandler.h --- a/Core/DicomNetworking/IGetRequestHandler.h Wed May 20 08:41:54 2020 +0200 +++ b/Core/DicomNetworking/IGetRequestHandler.h Wed May 20 09:19:35 2020 +0200 @@ -66,15 +66,15 @@ virtual Status DoNext(T_ASC_Association *) = 0; - virtual unsigned int nRemaining() = 0; + virtual unsigned int nRemaining() const = 0; - virtual unsigned int nCompleted() = 0; + virtual unsigned int nCompleted() const = 0; - virtual unsigned int warningCount() = 0; + virtual unsigned int warningCount() const = 0; - virtual const char * failedUids() = 0; + virtual const std::string& failedUids() const = 0; - virtual unsigned int nFailed() = 0; + virtual unsigned int nFailed() const = 0; }; } diff -r 4f09a6a2b369 -r 620e87e9e816 Core/DicomNetworking/Internals/CommandDispatcher.cpp --- a/Core/DicomNetworking/Internals/CommandDispatcher.cpp Wed May 20 08:41:54 2020 +0200 +++ b/Core/DicomNetworking/Internals/CommandDispatcher.cpp Wed May 20 09:19:35 2020 +0200 @@ -885,7 +885,7 @@ if (handler.get() != NULL) { - cond = Internals::getScp(assoc_, &msg, presID, *handler, remoteIp_, remoteAet_, calledAet_); + cond = Internals::getScp(assoc_, &msg, presID, *handler, remoteIp_, remoteAet_, calledAet_, associationTimeout_); } } break; diff -r 4f09a6a2b369 -r 620e87e9e816 Core/DicomNetworking/Internals/GetScp.cpp --- a/Core/DicomNetworking/Internals/GetScp.cpp Wed May 20 08:41:54 2020 +0200 +++ b/Core/DicomNetworking/Internals/GetScp.cpp Wed May 20 09:19:35 2020 +0200 @@ -40,44 +40,44 @@ Program: DCMTK 3.6.0 Module: http://dicom.offis.de/dcmtk.php.en -Copyright (C) 1994-2011, OFFIS e.V. -All rights reserved. + Copyright (C) 1994-2011, OFFIS e.V. + All rights reserved. -This software and supporting documentation were developed by + This software and supporting documentation were developed by OFFIS e.V. R&D Division Health Escherweg 2 26121 Oldenburg, Germany -Redistribution and use in source and binary forms, with or without -modification, are permitted provided that the following conditions -are met: + Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions + are met: -- Redistributions of source code must retain the above copyright + - Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. -- Redistributions in binary form must reproduce the above copyright + - Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. -- Neither the name of OFFIS nor the names of its contributors may be + - Neither the name of OFFIS nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission. -THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -=========================================================================*/ + =========================================================================*/ #include "../../PrecompiledHeaders.h" @@ -123,23 +123,27 @@ }; }; - void BuildFailedInstanceList(const char* failedUIDs, DcmDataset ** rspIds) + static DcmDataset *BuildFailedInstanceList(const std::string& failedUIDs) { - OFBool ok; - - if (failedUIDs != NULL) + if (failedUIDs.empty()) + { + return NULL; + } + else { - *rspIds = new DcmDataset(); - ok = DU_putStringDOElement(*rspIds, DCM_FailedSOPInstanceUIDList, failedUIDs); - if (!ok) + std::unique_ptr rspIds(new DcmDataset()); + + if (!DU_putStringDOElement(rspIds.get(), DCM_FailedSOPInstanceUIDList, failedUIDs.c_str())) { - LOG (ERROR) << "getSCP: failed to build DCM_FailedSOPInstanceUIDList"; + throw OrthancException(ErrorCode_InternalError, + "getSCP: failed to build DCM_FailedSOPInstanceUIDList"); } - + + return rspIds.release(); } } - void GetScpCallback( + static void GetScpCallback( /* in */ void *callbackData, OFBool cancelled, @@ -164,7 +168,7 @@ try { if(!data.handler_->Handle(input, data.remoteIp_, data.remoteAet_, - data.calledAet_)) + data.calledAet_)) { response->DimseStatus = STATUS_GET_Failed_UnableToProcess; return; @@ -223,21 +227,24 @@ { response->DimseStatus = STATUS_GET_Failed_UnableToProcess; - if (data.handler_->nFailed() > 0 || data.handler_->warningCount() > 0) + if (data.handler_->nFailed() > 0 || + data.handler_->warningCount() > 0) { response->DimseStatus = STATUS_GET_Warning_SubOperationsCompleteOneOrMoreFailures; } /* - * if all the sub-operations failed then we need to generate a failed or refused status. - * cf. DICOM part 4, C.4.3.3.1 - * we choose to generate a "Refused - Out of Resources - Unable to perform suboperations" status. + * if all the sub-operations failed then we need to generate + * a failed or refused status. cf. DICOM part 4, C.4.3.3.1 + * we choose to generate a "Refused - Out of Resources - + * Unable to perform suboperations" status. */ - if ((data.handler_->nFailed() > 0) && ((data.handler_->nCompleted() + data.handler_->warningCount()) == 0)) + if ((data.handler_->nFailed() > 0) && + ((data.handler_->nCompleted() + data.handler_->warningCount()) == 0)) { response->DimseStatus = STATUS_GET_Refused_OutOfResourcesSubOperations; } - BuildFailedInstanceList(data.handler_->failedUids(), responseIdentifiers); + *responseIdentifiers = BuildFailedInstanceList(data.handler_->failedUids()); } } @@ -246,18 +253,17 @@ response->NumberOfFailedSubOperations = data.handler_->nFailed(); response->NumberOfWarningSubOperations = data.handler_->warningCount(); } - } OFCondition Internals::getScp(T_ASC_Association * assoc, - T_DIMSE_Message * msg, - T_ASC_PresentationContextID presID, - IGetRequestHandler& handler, - std::string remoteIp, - std::string remoteAet, - std::string calledAet) + T_DIMSE_Message * msg, + T_ASC_PresentationContextID presID, + IGetRequestHandler& handler, + std::string remoteIp, + std::string remoteAet, + std::string calledAet, + int timeout) { - GetScpData data; data.lastRequest_ = NULL; data.handler_ = &handler; @@ -266,12 +272,11 @@ data.remoteAet_ = remoteAet; data.calledAet_ = calledAet; - OFCondition cond = DIMSE_getProvider(assoc, presID, &msg->msg.CGetRQ, - GetScpCallback, &data, - /*opt_blockMode*/ DIMSE_BLOCKING, - /*opt_dimse_timeout*/ 0); - + GetScpCallback, &data, + /*opt_blockMode*/ (timeout ? DIMSE_NONBLOCKING : DIMSE_BLOCKING), + /*opt_dimse_timeout*/ timeout); + // if some error occured, dump corresponding information and remove the outfile if necessary if (cond.bad()) { diff -r 4f09a6a2b369 -r 620e87e9e816 Core/DicomNetworking/Internals/GetScp.h --- a/Core/DicomNetworking/Internals/GetScp.h Wed May 20 08:41:54 2020 +0200 +++ b/Core/DicomNetworking/Internals/GetScp.h Wed May 20 09:19:35 2020 +0200 @@ -37,7 +37,6 @@ namespace Orthanc { - namespace Internals { OFCondition getScp(T_ASC_Association * assoc, @@ -46,6 +45,7 @@ IGetRequestHandler& handler, std::string remoteIp, std::string remoteAet, - std::string calledAet); + std::string calledAet, + int timeout); } } diff -r 4f09a6a2b369 -r 620e87e9e816 OrthancServer/OrthancGetRequestHandler.cpp --- a/OrthancServer/OrthancGetRequestHandler.cpp Wed May 20 08:41:54 2020 +0200 +++ b/OrthancServer/OrthancGetRequestHandler.cpp Wed May 20 09:19:35 2020 +0200 @@ -71,36 +71,28 @@ } const std::string& id = instances_[position_++]; - + std::string dicom; context_.ReadDicom(dicom, id); - if(dicom.size() <= 0) + if (dicom.size() <= 0) { return Status_Failure; } - - DcmInputBufferStream is; - is.setBuffer(&dicom[0], dicom.size()); - is.setEos(); - - DcmFileFormat dcmff; - OFCondition cond = dcmff.read(is, EXS_Unknown, EGL_noChange, DCM_MaxReadLength); - if(cond.bad()) - { - return Status_Failure; - } + + std::unique_ptr parsed( + FromDcmtkBridge::LoadFromMemoryBuffer(dicom.c_str(), dicom.size())); // Determine the storage SOP class UID for this instance DIC_UI sopClass; DIC_UI sopInstance; #if DCMTK_VERSION_NUMBER >= 364 - if (!DU_findSOPClassAndInstanceInDataSet(static_cast (dcmff.getDataset()), + if (!DU_findSOPClassAndInstanceInDataSet(static_cast (parsed->getDataset()), sopClass, sizeof(sopClass), sopInstance, sizeof(sopInstance))) #else - if (!DU_findSOPClassAndInstanceInDataSet(dcmff.getDataset(), sopClass, sopInstance)) + if (!DU_findSOPClassAndInstanceInDataSet(parsed->getDataset(), sopClass, sopInstance)) #endif { throw OrthancException(ErrorCode_NoSopClassOrInstance, @@ -108,42 +100,30 @@ originatorAet_); } - - - - cond = performGetSubOp(assoc, sopClass, sopInstance, dcmff.getDataset()); - if (getCancelled_) { - LOG (INFO) << "Get SCP: Received C-Cancel RQ"; + OFCondition cond = performGetSubOp(assoc, sopClass, sopInstance, parsed->getDataset()); + if (getCancelled_) + { + LOG(INFO) << "Get SCP: Received C-Cancel RQ"; } - if(cond.bad() || getCancelled_) + if (cond.bad() || getCancelled_) { return Status_Failure; } return Status_Success; } + void OrthancGetRequestHandler::addFailedUIDInstance(const char *sopInstance) { - size_t len; - - if (failedUIDs_ == NULL) { - if ((failedUIDs_ = (char*)malloc(DIC_UI_LEN+1)) == NULL) { - LOG (ERROR) << "malloc failure: addFailedUIDInstance"; - return; - } - strcpy(failedUIDs_, sopInstance); - } else { - len = strlen(failedUIDs_); - if ((failedUIDs_ = (char*)realloc(failedUIDs_, - (len+strlen(sopInstance)+2))) == NULL) { - LOG (ERROR) << "realloc failure: addFailedUIDInstance"; - return; - } - // tag sopInstance onto end of old with '\' between - strcat(failedUIDs_, "\\"); - strcat(failedUIDs_, sopInstance); + if (failedUIDs_.empty()) + { + failedUIDs_ = sopInstance; + } + else + { + failedUIDs_ += "\\" + std::string(sopInstance); } } @@ -160,19 +140,21 @@ T_ASC_PresentationContextID presId; DcmDataset *stDetail = NULL; - msgId = assoc->nextMsgID++; // which presentation context should be used - presId = ASC_findAcceptedPresentationContextID(assoc, - sopClass); - if (presId == 0) { + presId = ASC_findAcceptedPresentationContextID(assoc, sopClass); + + if (presId == 0) + { nFailed_++; addFailedUIDInstance(sopInstance); - LOG (ERROR) << "Get SCP: storeSCU: No presentation context for: (" - << dcmSOPClassUIDToModality(sopClass, "OT") << ") " << sopClass; + LOG(ERROR) << "Get SCP: storeSCU: No presentation context for: (" + << dcmSOPClassUIDToModality(sopClass, "OT") << ") " << sopClass; return DIMSE_NOVALIDPRESENTATIONCONTEXTID; - } else { + } + else + { // make sure that we can send images in this presentation context T_ASC_PresentationContext pc; ASC_findAcceptedPresentationContext(assoc->params, presId, &pc); @@ -182,8 +164,8 @@ // the role is not appropriate nFailed_++; addFailedUIDInstance(sopInstance); - LOG (ERROR) <<"Get SCP: storeSCU: [No presentation context with requestor SCP role for: (" - << dcmSOPClassUIDToModality(sopClass, "OT") << ") " << sopClass; + LOG(ERROR) <<"Get SCP: storeSCU: [No presentation context with requestor SCP role for: (" + << dcmSOPClassUIDToModality(sopClass, "OT") << ") " << sopClass; return DIMSE_NOVALIDPRESENTATIONCONTEXTID; } } @@ -195,8 +177,8 @@ req.Priority = priority_; req.opts = 0; - LOG (INFO) << "Store SCU RQ: MsgID " << msgId << ", (" - << dcmSOPClassUIDToModality(sopClass, "OT") << ")"; + LOG(INFO) << "Store SCU RQ: MsgID " << msgId << ", (" + << dcmSOPClassUIDToModality(sopClass, "OT") << ")"; T_DIMSE_DetectedCancelParameters cancelParameters; @@ -204,47 +186,63 @@ NULL, dataset, getSubOpProgressCallback, this, DIMSE_BLOCKING, 0, &rsp, &stDetail, &cancelParameters); - if (cond.good()) { - if (cancelParameters.cancelEncountered) { + if (cond.good()) + { + if (cancelParameters.cancelEncountered) + { if (origPresId == cancelParameters.presId && - origMsgId == cancelParameters.req.MessageIDBeingRespondedTo) { + origMsgId == cancelParameters.req.MessageIDBeingRespondedTo) + { getCancelled_ = OFTrue; - } else { - LOG (ERROR) << "Get SCP: Unexpected C-Cancel-RQ encountered: pid=" << (int)cancelParameters.presId - << ", mid=" << (int)cancelParameters.req.MessageIDBeingRespondedTo; + } + else + { + LOG(ERROR) << "Get SCP: Unexpected C-Cancel-RQ encountered: pid=" << (int)cancelParameters.presId + << ", mid=" << (int)cancelParameters.req.MessageIDBeingRespondedTo; } } - if (rsp.DimseStatus == STATUS_Success) { + if (rsp.DimseStatus == STATUS_Success) + { // everything ok nCompleted_++; - } else if ((rsp.DimseStatus & 0xf000) == 0xb000) { + } + else if ((rsp.DimseStatus & 0xf000) == 0xb000) + { // a warning status message warningCount_++; - LOG (ERROR) << "Get SCP: Store Warning: Response Status: " << DU_cstoreStatusString(rsp.DimseStatus); - } else { + LOG(ERROR) << "Get SCP: Store Warning: Response Status: " + << DU_cstoreStatusString(rsp.DimseStatus); + } + else + { nFailed_++; addFailedUIDInstance(sopInstance); // print a status message - LOG (ERROR) << "Get SCP: Store Failed: Response Status: " - << DU_cstoreStatusString(rsp.DimseStatus); + LOG(ERROR) << "Get SCP: Store Failed: Response Status: " + << DU_cstoreStatusString(rsp.DimseStatus); } - } else { + } + else + { nFailed_++; addFailedUIDInstance(sopInstance); OFString temp_str; - LOG (ERROR) << "Get SCP: storeSCU: Store Request Failed: " << DimseCondition::dump(temp_str, cond); + LOG(ERROR) << "Get SCP: storeSCU: Store Request Failed: " << DimseCondition::dump(temp_str, cond); } - if (stDetail) { - LOG (INFO) << " Status Detail:" << OFendl << DcmObject::PrintHelper(*stDetail); + + if (stDetail) + { + LOG(INFO) << " Status Detail:" << OFendl << DcmObject::PrintHelper(*stDetail); delete stDetail; } + return cond; } bool OrthancGetRequestHandler::LookupIdentifiers(std::vector& publicIds, - ResourceType level, - const DicomMap& input) + ResourceType level, + const DicomMap& input) { DicomTag tag(0, 0); // Dummy initialization @@ -291,6 +289,17 @@ } + OrthancGetRequestHandler::OrthancGetRequestHandler(ServerContext& context) : + context_(context) + { + position_ = 0; + nRemaining_ = 0; + nCompleted_ = 0; + warningCount_ = 0; + nFailed_ = 0; + } + + bool OrthancGetRequestHandler::Handle(const DicomMap& input, const std::string& originatorIp, const std::string& originatorAet, @@ -342,7 +351,7 @@ for (size_t i = 0; i < publicIds.size(); i++) { LOG(INFO) << "Sending resource " << publicIds[i] << " to modality \"" - << originatorAet << "\" in synchronous mode"; + << originatorAet << "\" in synchronous mode"; std::list tmp; context_.GetIndex().GetChildInstances(tmp, publicIds[i]); @@ -353,7 +362,8 @@ instances_.push_back(*it); } } - failedUIDs_ = NULL; + + failedUIDs_.clear(); getCancelled_ = OFFalse; nRemaining_ = GetSubOperationCount(); @@ -361,8 +371,6 @@ nFailed_ = 0; warningCount_ = 0; - return retVal; - - + return retVal; } }; diff -r 4f09a6a2b369 -r 620e87e9e816 OrthancServer/OrthancGetRequestHandler.h --- a/OrthancServer/OrthancGetRequestHandler.h Wed May 20 08:41:54 2020 +0200 +++ b/OrthancServer/OrthancGetRequestHandler.h Wed May 20 09:19:35 2020 +0200 @@ -55,7 +55,7 @@ unsigned int nCompleted_; unsigned int warningCount_; unsigned int nFailed_; - char *failedUIDs_; + std::string failedUIDs_; T_DIMSE_Priority priority_; DIC_US origMsgId; @@ -75,57 +75,43 @@ void addFailedUIDInstance(const char *sopInstance); public: - OrthancGetRequestHandler(ServerContext& context) : - context_(context) - { - position_ = 0; - - nRemaining_ = 0; - nCompleted_ = 0; - warningCount_ = 0; - nFailed_ = 0; - - failedUIDs_ = NULL; - } - + OrthancGetRequestHandler(ServerContext& context); + bool Handle(const DicomMap& input, const std::string& originatorIp, const std::string& originatorAet, const std::string& calledAet); - virtual Status DoNext(T_ASC_Association *); + virtual Status DoNext(T_ASC_Association *) ORTHANC_OVERRIDE; - virtual unsigned int GetSubOperationCount() const + virtual unsigned int GetSubOperationCount() const ORTHANC_OVERRIDE { return (unsigned int) instances_.size(); } - virtual unsigned int nRemaining() + virtual unsigned int nRemaining() const ORTHANC_OVERRIDE { return nRemaining_; } - virtual unsigned int nCompleted() + virtual unsigned int nCompleted() const ORTHANC_OVERRIDE { return nCompleted_; } - virtual unsigned int warningCount() + virtual unsigned int warningCount() const ORTHANC_OVERRIDE { return warningCount_; } - virtual unsigned int nFailed() + virtual unsigned int nFailed() const ORTHANC_OVERRIDE { return nFailed_; } - virtual const char * failedUids() + virtual const std::string& failedUids() const ORTHANC_OVERRIDE { return failedUIDs_; - } - - - + } }; }