Mercurial > hg > orthanc
changeset 5417:2a7a113d791d
merge
| author | Sebastien Jodogne <s.jodogne@gmail.com> |
|---|---|
| date | Wed, 08 Nov 2023 16:10:46 +0100 |
| parents | 08177310e269 (current diff) d5c15e9a63dd (diff) |
| children | cd90d8b4661e |
| files | |
| diffstat | 25 files changed, 399 insertions(+), 143 deletions(-) [+] |
line wrap: on
line diff
--- a/NEWS Wed Nov 08 09:59:31 2023 +0100 +++ b/NEWS Wed Nov 08 16:10:46 2023 +0100 @@ -4,10 +4,31 @@ General ------- +* Performance: + - Allow multiple plugins to use the plugin SDK at the same time. In previous versions, + functions like instance transcoding or instance reading where mutually exclusive. + This can bring some significant improvements particularly in viewers. * Housekeeper plugin: - Update to rebuild the cache of the DicomWeb plugin when updating to DicomWeb 1.15. - New trigger configuration: "DicomWebCacheChange" - Fixed reading the triggers configuration. +* HTTP Compression: + - The default value of the "HttpCompressionEnabled" is now false by default. This reduces + the Orthanc overall CPU usage and latency. This is suitable for setups with large + bandwidth network like LAN. + - When "HttpCompressionEnabled" is true, only the content that is clearly identified as + compressible is compressed (JSON, XML, HTML, text, ...). DICOM files are never + compressed over HTTP. In prior versions, all content types were compressed. + This notably greatly improve loading time of large DICOM + files through WADO-RS e.g in StoneViewer when working on large bandwidth networks. + - When "HttpCompressionEnabled" is true, content < 2KB are never compressed. + +Bug Fixes +--------- + +* Solved a deadlock related to the Job Engine events and plugins. Job events are now pushed + into a queue to be handled asynchronously by plugins. + REST API --------
--- a/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp Wed Nov 08 16:10:46 2023 +0100 @@ -1574,7 +1574,8 @@ static bool SaveToMemoryBufferInternal(std::string& buffer, DcmFileFormat& dicom, - E_TransferSyntax xfer) + E_TransferSyntax xfer, + std::string& errorMessage) { E_EncodingType encodingType = /*opt_sequenceType*/ EET_ExplicitLength; @@ -1613,14 +1614,24 @@ { // Error buffer.clear(); + errorMessage = std::string(c.text()); return false; } } - bool FromDcmtkBridge::SaveToMemoryBuffer(std::string& buffer, DcmDataset& dataSet) { + std::string errorMessageNotUsed; + return SaveToMemoryBuffer(buffer, dataSet, errorMessageNotUsed); + } + + + + bool FromDcmtkBridge::SaveToMemoryBuffer(std::string& buffer, + DcmDataset& dataSet, + std::string& errorMessage) + { // Determine the transfer syntax which shall be used to write the // information to the file. If not possible, switch to the Little // Endian syntax, with explicit length. @@ -1647,7 +1658,7 @@ ff.validateMetaInfo(xfer); ff.removeInvalidGroups(); - return SaveToMemoryBufferInternal(buffer, ff, xfer); + return SaveToMemoryBufferInternal(buffer, ff, xfer, errorMessage); }
--- a/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.h Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.h Wed Nov 08 16:10:46 2023 +0100 @@ -203,6 +203,10 @@ static bool SaveToMemoryBuffer(std::string& buffer, DcmDataset& dataSet); + static bool SaveToMemoryBuffer(std::string& buffer, + DcmDataset& dataSet, + std::string& errorMessage); + static bool Transcode(DcmFileFormat& dicom, DicomTransferSyntax syntax, const DcmRepresentationParameter* representation);
--- a/OrthancFramework/Sources/DicomParsing/ParsedDicomFile.cpp Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancFramework/Sources/DicomParsing/ParsedDicomFile.cpp Wed Nov 08 16:10:46 2023 +0100 @@ -939,9 +939,10 @@ void ParsedDicomFile::SaveToMemoryBuffer(std::string& buffer) { - if (!FromDcmtkBridge::SaveToMemoryBuffer(buffer, *GetDcmtkObject().getDataset())) + std::string errorMessage; + if (!FromDcmtkBridge::SaveToMemoryBuffer(buffer, *GetDcmtkObject().getDataset(), errorMessage) { - throw OrthancException(ErrorCode_InternalError, "Cannot write DICOM file to memory"); + throw OrthancException(ErrorCode_InternalError, "Cannot write DICOM file to memory, DCMTK error: " + errorMessage); } }
--- a/OrthancFramework/Sources/Enumerations.cpp Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancFramework/Sources/Enumerations.cpp Wed Nov 08 16:10:46 2023 +0100 @@ -35,26 +35,6 @@ namespace Orthanc { - static const char* const MIME_CSS = "text/css"; - static const char* const MIME_DICOM = "application/dicom"; - static const char* const MIME_GIF = "image/gif"; - static const char* const MIME_GZIP = "application/gzip"; - static const char* const MIME_HTML = "text/html"; - static const char* const MIME_JAVASCRIPT = "application/javascript"; - static const char* const MIME_JPEG2000 = "image/jp2"; - static const char* const MIME_NACL = "application/x-nacl"; - static const char* const MIME_PLAIN_TEXT = "text/plain"; - static const char* const MIME_PNACL = "application/x-pnacl"; - static const char* const MIME_SVG = "image/svg+xml"; - static const char* const MIME_WEB_ASSEMBLY = "application/wasm"; - static const char* const MIME_WOFF = "application/x-font-woff"; - static const char* const MIME_WOFF2 = "font/woff2"; - static const char* const MIME_XML_2 = "text/xml"; - static const char* const MIME_ZIP = "application/zip"; - static const char* const MIME_DICOM_WEB_JSON = "application/dicom+json"; - static const char* const MIME_DICOM_WEB_XML = "application/dicom+xml"; - static const char* const MIME_ICO = "image/x-icon"; - // This function is autogenerated by the script // "Resources/CodeGeneration/GenerateErrorCodes.py" const char* EnumerationToString(ErrorCode error)
--- a/OrthancFramework/Sources/Enumerations.h Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancFramework/Sources/Enumerations.h Wed Nov 08 16:10:46 2023 +0100 @@ -47,6 +47,27 @@ static const char* const MIME_MTL = "model/mtl"; static const char* const MIME_STL = "model/stl"; + static const char* const MIME_CSS = "text/css"; + static const char* const MIME_DICOM = "application/dicom"; + static const char* const MIME_GIF = "image/gif"; + static const char* const MIME_GZIP = "application/gzip"; + static const char* const MIME_HTML = "text/html"; + static const char* const MIME_JAVASCRIPT = "application/javascript"; + static const char* const MIME_JPEG2000 = "image/jp2"; + static const char* const MIME_NACL = "application/x-nacl"; + static const char* const MIME_PLAIN_TEXT = "text/plain"; + static const char* const MIME_PNACL = "application/x-pnacl"; + static const char* const MIME_SVG = "image/svg+xml"; + static const char* const MIME_WEB_ASSEMBLY = "application/wasm"; + static const char* const MIME_WOFF = "application/x-font-woff"; + static const char* const MIME_WOFF2 = "font/woff2"; + static const char* const MIME_XML_2 = "text/xml"; + static const char* const MIME_ZIP = "application/zip"; + static const char* const MIME_DICOM_WEB_JSON = "application/dicom+json"; + static const char* const MIME_DICOM_WEB_XML = "application/dicom+xml"; + static const char* const MIME_ICO = "image/x-icon"; + + /** * "No Internet Media Type (aka MIME type, content type) for PBM has * been registered with IANA, but the unofficial value
--- a/OrthancFramework/Sources/HttpServer/FilesystemHttpHandler.cpp Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancFramework/Sources/HttpServer/FilesystemHttpHandler.cpp Wed Nov 08 16:10:46 2023 +0100 @@ -153,7 +153,7 @@ { FilesystemHttpSender sender(p); sender.SetContentType(SystemToolbox::AutodetectMimeType(p.string())); - output.Answer(sender); // TODO COMPRESSION + output.Answer(sender); } else if (listDirectoryContent_ && fs::exists(p) &&
--- a/OrthancFramework/Sources/HttpServer/HttpFileSender.cpp Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancFramework/Sources/HttpServer/HttpFileSender.cpp Wed Nov 08 16:10:46 2023 +0100 @@ -53,7 +53,8 @@ if (contentType_.empty()) { - contentType_ = SystemToolbox::AutodetectMimeType(filename); + MimeType mimeType = SystemToolbox::AutodetectMimeType(filename); + contentType_ = EnumerationToString(mimeType); } }
--- a/OrthancFramework/Sources/HttpServer/HttpOutput.cpp Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancFramework/Sources/HttpServer/HttpOutput.cpp Wed Nov 08 16:10:46 2023 +0100 @@ -30,6 +30,7 @@ #include "../Logging.h" #include "../OrthancException.h" #include "../Toolbox.h" +#include "../SystemToolbox.h" #include <iostream> #include <vector> @@ -51,6 +52,7 @@ unsigned int keepAliveTimeout) : stream_(stream), state_(State_WritingHeader), + isContentCompressible_(false), status_(HttpStatus_200_Ok), hasContentLength_(false), contentLength_(0), @@ -102,6 +104,17 @@ AddHeader("Content-Type", contentType); } + void HttpOutput::StateMachine::SetContentCompressible(bool isContentCompressible) + { + isContentCompressible_ = isContentCompressible; + } + + bool HttpOutput::StateMachine::IsContentCompressible() const + { + // We assume that all files that compress correctly (mainly JSON, XML) are clearly identified. + return isContentCompressible_; + } + void HttpOutput::StateMachine::SetContentFilename(const char* filename) { // TODO Escape double quotes @@ -275,13 +288,11 @@ HttpCompression HttpOutput::GetPreferredCompression(size_t bodySize) const { -#if 0 - // TODO Do not compress small files? - if (bodySize < 512) + // Do not compress small files since there is no real size benefit. + if (bodySize < 2048) { return HttpCompression_None; } -#endif // Prefer "gzip" over "deflate" if the choice is offered @@ -368,11 +379,13 @@ void HttpOutput::SetContentType(MimeType contentType) { stateMachine_.SetContentType(EnumerationToString(contentType)); + stateMachine_.SetContentCompressible(SystemToolbox::IsContentCompressible(contentType)); } void HttpOutput::SetContentType(const std::string &contentType) { stateMachine_.SetContentType(contentType.c_str()); + stateMachine_.SetContentCompressible(SystemToolbox::IsContentCompressible(contentType)); } void HttpOutput::SetContentFilename(const char *filename) @@ -442,7 +455,7 @@ HttpCompression compression = GetPreferredCompression(length); - if (compression == HttpCompression_None) + if (compression == HttpCompression_None || !IsContentCompressible()) { stateMachine_.SetContentLength(length); stateMachine_.SendBody(buffer, length);
--- a/OrthancFramework/Sources/HttpServer/HttpOutput.h Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancFramework/Sources/HttpServer/HttpOutput.h Wed Nov 08 16:10:46 2023 +0100 @@ -56,6 +56,7 @@ IHttpOutputStream& stream_; State state_; + bool isContentCompressible_; HttpStatus status_; bool hasContentLength_; uint64_t contentLength_; @@ -82,6 +83,8 @@ void SetContentType(const char* contentType); + void SetContentCompressible(bool isCompressible); + void SetContentFilename(const char* filename); void SetCookie(const std::string& cookie, @@ -110,6 +113,8 @@ return state_; } + bool IsContentCompressible() const; + void CheckHeadersCompatibilityWithMultipart() const; void StartStream(const std::string& contentType); @@ -139,6 +144,11 @@ bool IsGzipAllowed() const; + bool IsContentCompressible() const + { + return stateMachine_.IsContentCompressible(); + } + void SendStatus(HttpStatus status, const char* message, size_t messageSize);
--- a/OrthancFramework/Sources/SystemToolbox.cpp Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancFramework/Sources/SystemToolbox.cpp Wed Nov 08 16:10:46 2023 +0100 @@ -725,6 +725,52 @@ } } + bool SystemToolbox::IsContentCompressible(MimeType mime) + { + switch (mime) + { + case MimeType_Css: + case MimeType_Html: + case MimeType_JavaScript: + case MimeType_Json: + case MimeType_Pam: + case MimeType_Pdf: + case MimeType_PlainText: + case MimeType_WebAssembly: + case MimeType_Xml: + case MimeType_PrometheusText: + case MimeType_DicomWebJson: + case MimeType_DicomWebXml: + return true; + default: // for all other (JPEG, DICOM, binary, ...) + return false; + } + } + + bool SystemToolbox::IsContentCompressible(const std::string& contentType) + { + if (contentType.empty()) + { + return false; + } + + if (contentType.find(MIME_JSON) != std::string::npos || + contentType.find(MIME_XML) != std::string::npos || + contentType.find(MIME_DICOM_WEB_JSON) != std::string::npos || + contentType.find(MIME_DICOM_WEB_XML) != std::string::npos || + contentType.find(MIME_PDF) != std::string::npos || + contentType.find(MIME_CSS) != std::string::npos || + contentType.find(MIME_HTML) != std::string::npos || + contentType.find(MIME_JAVASCRIPT) != std::string::npos || + contentType.find(MIME_PLAIN_TEXT) != std::string::npos || + contentType.find(MIME_WEB_ASSEMBLY) != std::string::npos || + contentType.find(MIME_XML_2) != std::string::npos) + { + return true; + } + + return false; + } MimeType SystemToolbox::AutodetectMimeType(const std::string& path) {
--- a/OrthancFramework/Sources/SystemToolbox.h Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancFramework/Sources/SystemToolbox.h Wed Nov 08 16:10:46 2023 +0100 @@ -108,6 +108,10 @@ static unsigned int GetHardwareConcurrency(); + static bool IsContentCompressible(MimeType mime); + + static bool IsContentCompressible(const std::string& contentType); + static MimeType AutodetectMimeType(const std::string& path); static void GetEnvironmentVariables(std::map<std::string, std::string>& env);
--- a/OrthancServer/Plugins/Engine/OrthancPlugins.cpp Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancServer/Plugins/Engine/OrthancPlugins.cpp Wed Nov 08 16:10:46 2023 +0100 @@ -1090,7 +1090,10 @@ class OrthancPlugins::PImpl { private: - boost::mutex contextMutex_; + boost::mutex contextMutex_; + boost::condition_variable contextCond_; + size_t contextRefCount_; + ServerContext* context_; public: @@ -1458,21 +1461,38 @@ }; - class ServerContextLock + // This class ensures that the Context remains valid while being used. + // But it does not prevent multiple users to use the context at the same time. + // (new behavior in 1.12.2. In previous version, only one user could use the "locked" context) + class ServerContextReference { private: - boost::mutex::scoped_lock lock_; ServerContext* context_; + boost::mutex& mutex_; + boost::condition_variable& cond_; + size_t& refCount_; public: - explicit ServerContextLock(PImpl& that) : - lock_(that.contextMutex_), - context_(that.context_) + explicit ServerContextReference(PImpl& that) : + context_(that.context_), + mutex_(that.contextMutex_), + cond_(that.contextCond_), + refCount_(that.contextRefCount_) { if (context_ == NULL) { throw OrthancException(ErrorCode_DatabaseNotInitialized); } + + boost::mutex::scoped_lock lock(mutex_); + refCount_++; + } + + ~ServerContextReference() + { + boost::mutex::scoped_lock lock(mutex_); + refCount_--; + cond_.notify_one(); } ServerContext& GetContext() @@ -1485,7 +1505,13 @@ void SetServerContext(ServerContext* context) { + // update only the context while nobody is using it boost::mutex::scoped_lock lock(contextMutex_); + + while (contextRefCount_ > 0) + { + contextCond_.wait(lock); + } context_ = context; } @@ -1554,6 +1580,7 @@ unsigned int maxDatabaseRetries_; // New in Orthanc 1.9.2 explicit PImpl(const std::string& databaseServerIdentifier) : + contextRefCount_(0), context_(NULL), findCallback_(NULL), worklistCallback_(NULL), @@ -1600,7 +1627,7 @@ { static const char* LUA_CALLBACK = "IncomingWorklistRequestFilter"; - PImpl::ServerContextLock lock(*that_.pimpl_); + PImpl::ServerContextReference lock(*that_.pimpl_); LuaScripting::Lock lua(lock.GetContext().GetLuaScripting()); if (!lua.GetLua().IsExistingFunction(LUA_CALLBACK)) @@ -2730,6 +2757,25 @@ } + void OrthancPlugins::SignalJobEvent(const JobEvent& event) + { + // job events are actually considered as changes inside plugins -> translate + switch (event.GetEventType()) + { + case JobEventType_Submitted: + SignalChangeInternal(OrthancPluginChangeType_JobSubmitted, OrthancPluginResourceType_None, event.GetJobId().c_str()); + break; + case JobEventType_Success: + SignalChangeInternal(OrthancPluginChangeType_JobSuccess, OrthancPluginResourceType_None, event.GetJobId().c_str()); + break; + case JobEventType_Failure: + SignalChangeInternal(OrthancPluginChangeType_JobFailure, OrthancPluginResourceType_None, event.GetJobId().c_str()); + break; + default: + throw OrthancException(ErrorCode_InternalError); + } + } + void OrthancPlugins::RegisterRestCallback(const void* parameters, bool mutualExclusion) @@ -2776,6 +2822,8 @@ void OrthancPlugins::RegisterOnChangeCallback(const void* parameters) { + boost::recursive_mutex::scoped_lock lock(pimpl_->changeCallbackMutex_); + const _OrthancPluginOnChangeCallback& p = *reinterpret_cast<const _OrthancPluginOnChangeCallback*>(parameters); @@ -3135,7 +3183,7 @@ std::string dicom; { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); lock.GetContext().ReadDicom(dicom, p.instanceId); } @@ -3176,7 +3224,7 @@ IHttpHandler* handler; { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); handler = &lock.GetContext().GetHttpHandler().RestrictToOrthancRestApi(!afterPlugins); } @@ -3209,7 +3257,7 @@ IHttpHandler* handler; { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); handler = &lock.GetContext().GetHttpHandler().RestrictToOrthancRestApi(!p.afterPlugins); } @@ -3233,7 +3281,7 @@ IHttpHandler* handler; { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); handler = &lock.GetContext().GetHttpHandler().RestrictToOrthancRestApi(!afterPlugins); } @@ -3260,7 +3308,7 @@ IHttpHandler* handler; { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); handler = &lock.GetContext().GetHttpHandler().RestrictToOrthancRestApi(!afterPlugins); } @@ -3319,7 +3367,7 @@ std::vector<std::string> result; { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); lock.GetContext().GetIndex().LookupIdentifierExact(result, level, tag, p.argument); } @@ -3614,7 +3662,7 @@ std::unique_ptr<ImageAccessor> decoded; { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); decoded.reset(lock.GetContext().DecodeDicomFrame(instance, p.frameIndex)); } @@ -3697,7 +3745,7 @@ case OrthancPluginImageFormat_Dicom: { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); image.reset(lock.GetContext().DecodeDicomFrame(p.data, p.size, 0)); break; } @@ -4022,7 +4070,7 @@ IHttpHandler* handler; { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); handler = &lock.GetContext().GetHttpHandler().RestrictToOrthancRestApi(!p.afterPlugins); } @@ -4244,7 +4292,7 @@ std::string content; { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); lock.GetContext().ReadDicom(content, p.instanceId); } @@ -4373,7 +4421,7 @@ case _OrthancPluginService_DecodeDicomImage: { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); result.reset(lock.GetContext().DecodeDicomFrame(p.constBuffer, p.bufferSize, p.frameIndex)); break; } @@ -4426,7 +4474,7 @@ std::string buffer; { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); lock.GetContext().ReadDicom(buffer, params.instanceId); } @@ -4443,7 +4491,7 @@ std::string buffer; { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); if (!lock.GetContext().ReadDicomUntilPixelData(buffer, params.instanceId)) { lock.GetContext().ReadDicom(buffer, params.instanceId); @@ -4461,7 +4509,7 @@ ValueRepresentation pixelDataVR = parsed->GuessPixelDataValueRepresentation(); { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); std::string s; int64_t revision; // unused @@ -4792,7 +4840,7 @@ { // TODO - Plugins can only access global properties of their // own Orthanc server (no access to the shared global properties) - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); lock.GetContext().GetIndex().SetGlobalProperty(static_cast<GlobalProperty>(p.property), false /* not shared */, p.value); return true; @@ -4809,7 +4857,7 @@ { // TODO - Plugins can only access global properties of their // own Orthanc server (no access to the shared global properties) - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); result = lock.GetContext().GetIndex().GetGlobalProperty(static_cast<GlobalProperty>(p.property), false /* not shared */, p.value); } @@ -5276,7 +5324,7 @@ std::string uuid; - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); lock.GetContext().GetJobsEngine().GetRegistry().Submit (uuid, reinterpret_cast<PluginsJob*>(p.job), p.priority); @@ -5299,7 +5347,7 @@ *reinterpret_cast<const _OrthancPluginSetMetricsValue*>(parameters); { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); lock.GetContext().GetMetricsRegistry().SetFloatValue(p.name, p.value, Plugins::Convert(p.type)); } @@ -5312,7 +5360,7 @@ *reinterpret_cast<const _OrthancPluginSetMetricsIntegerValue*>(parameters); { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); lock.GetContext().GetMetricsRegistry().SetIntegerValue(p.name, p.value, Plugins::Convert(p.type)); } @@ -5404,7 +5452,7 @@ bool success; { - PImpl::ServerContextLock lock(*pimpl_); + PImpl::ServerContextReference lock(*pimpl_); success = lock.GetContext().Transcode( transcoded, source, syntaxes, true /* allow new sop */); }
--- a/OrthancServer/Plugins/Engine/OrthancPlugins.h Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancServer/Plugins/Engine/OrthancPlugins.h Wed Nov 08 16:10:46 2023 +0100 @@ -269,7 +269,9 @@ const void* parameters) ORTHANC_OVERRIDE; virtual void SignalChange(const ServerIndexChange& change) ORTHANC_OVERRIDE; - + + virtual void SignalJobEvent(const JobEvent& event) ORTHANC_OVERRIDE; + virtual void SignalStoredInstance(const std::string& instanceId, const DicomInstanceToStore& instance, const Json::Value& simplifiedTags) ORTHANC_OVERRIDE; @@ -319,21 +321,6 @@ SignalChangeInternal(OrthancPluginChangeType_OrthancStopped, OrthancPluginResourceType_None, NULL); } - void SignalJobSubmitted(const std::string& jobId) - { - SignalChangeInternal(OrthancPluginChangeType_JobSubmitted, OrthancPluginResourceType_None, jobId.c_str()); - } - - void SignalJobSuccess(const std::string& jobId) - { - SignalChangeInternal(OrthancPluginChangeType_JobSuccess, OrthancPluginResourceType_None, jobId.c_str()); - } - - void SignalJobFailure(const std::string& jobId) - { - SignalChangeInternal(OrthancPluginChangeType_JobFailure, OrthancPluginResourceType_None, jobId.c_str()); - } - void SignalUpdatedPeers() { SignalChangeInternal(OrthancPluginChangeType_UpdatedPeers, OrthancPluginResourceType_None, NULL);
--- a/OrthancServer/Plugins/Samples/Common/OrthancPluginCppWrapper.cpp Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancServer/Plugins/Samples/Common/OrthancPluginCppWrapper.cpp Wed Nov 08 16:10:46 2023 +0100 @@ -79,6 +79,10 @@ } } + void ResetGlobalContext() + { + globalContext_ = NULL; + } bool HasGlobalContext() {
--- a/OrthancServer/Plugins/Samples/Common/OrthancPluginCppWrapper.h Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancServer/Plugins/Samples/Common/OrthancPluginCppWrapper.h Wed Nov 08 16:10:46 2023 +0100 @@ -137,6 +137,8 @@ void SetGlobalContext(OrthancPluginContext* context); + void ResetGlobalContext(); + bool HasGlobalContext(); OrthancPluginContext* GetGlobalContext();
--- a/OrthancServer/Resources/Configuration.json Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancServer/Resources/Configuration.json Wed Nov 08 16:10:46 2023 +0100 @@ -125,7 +125,12 @@ // Enable HTTP compression to improve network bandwidth utilization, // at the expense of more computations on the server. Orthanc // supports the "gzip" and "deflate" HTTP encodings. - "HttpCompressionEnabled" : true, + // When working on a LAN or on localhost, you should typically set + // this configuration to false while when working on low-bandwidth, + // you should set it to true. + // Note in versions up to 1.12.1, the default value was "true" and is + // "false" since 1.12.2. + "HttpCompressionEnabled" : false, // Enable the publication of the content of the Orthanc server as a // WebDAV share (new in Orthanc 1.8.0). On the localhost, the WebDAV @@ -244,7 +249,7 @@ "SslVerifyPeers" : false, // Path to a file containing the concatenation of the client SSL - // certificate(s) that are trusted to verify the identify of remote + // certificate(s) that are trusted to verify the identity of remote // HTTP clients. The individual certificate(s) must be stored in the // PEM format. This option is only meaningful if "SslVerifyPeers" // is true.
--- a/OrthancServer/Sources/IServerListener.h Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancServer/Sources/IServerListener.h Wed Nov 08 16:10:46 2023 +0100 @@ -24,6 +24,7 @@ #include "DicomInstanceToStore.h" #include "ServerIndexChange.h" +#include "JobEvent.h" #include <json/value.h> @@ -42,6 +43,8 @@ virtual void SignalChange(const ServerIndexChange& change) = 0; + virtual void SignalJobEvent(const JobEvent& event) = 0; + virtual bool FilterIncomingInstance(const DicomInstanceToStore& instance, const Json::Value& simplified) = 0;
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/OrthancServer/Sources/JobEvent.h Wed Nov 08 16:10:46 2023 +0100 @@ -0,0 +1,77 @@ +/** + * Orthanc - A Lightweight, RESTful DICOM Store + * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics + * Department, University Hospital of Liege, Belgium + * Copyright (C) 2017-2023 Osimis S.A., Belgium + * Copyright (C) 2021-2023 Sebastien Jodogne, ICTEAM UCLouvain, Belgium + * + * This program is free software: you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + **/ + + +#pragma once + +#include "ServerEnumerations.h" +#include "../../OrthancFramework/Sources/IDynamicObject.h" +#include "../../OrthancFramework/Sources/SystemToolbox.h" + +#include <string> +#include <json/value.h> + +namespace Orthanc +{ + enum JobEventType + { + JobEventType_Failure, + JobEventType_Submitted, + JobEventType_Success + }; + + + struct JobEvent : public IDynamicObject + { + private: + JobEventType eventType_; + std::string jobId_; + + public: + JobEvent(JobEventType eventType, + const std::string& jobId) : + eventType_(eventType), + jobId_(jobId) + { + } + + JobEvent(const JobEvent& other) + : eventType_(other.eventType_), + jobId_(other.jobId_) + { + } + + // JobEvent* Clone() const + // { + // return new JobEvent(*this); + // } + + JobEventType GetEventType() const + { + return eventType_; + } + + const std::string& GetJobId() const + { + return jobId_; + } + }; +}
--- a/OrthancServer/Sources/LuaScripting.cpp Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancServer/Sources/LuaScripting.cpp Wed Nov 08 16:10:46 2023 +0100 @@ -239,25 +239,14 @@ }; - class LuaScripting::JobEvent : public LuaScripting::IEvent + class LuaScripting::LuaJobEvent : public LuaScripting::IEvent { - public: - enum Type - { - Type_Failure, - Type_Submitted, - Type_Success - }; - private: - Type type_; - std::string jobId_; + JobEvent event_; public: - JobEvent(Type type, - const std::string& jobId) : - type_(type), - jobId_(jobId) + LuaJobEvent(const JobEvent& event) : + event_(event) { } @@ -265,17 +254,17 @@ { std::string functionName; - switch (type_) + switch (event_.GetEventType()) { - case Type_Failure: + case JobEventType_Failure: functionName = "OnJobFailure"; break; - case Type_Submitted: + case JobEventType_Submitted: functionName = "OnJobSubmitted"; break; - case Type_Success: + case JobEventType_Success: functionName = "OnJobSuccess"; break; @@ -289,7 +278,7 @@ if (lock.GetLua().IsExistingFunction(functionName.c_str())) { LuaFunctionCall call(lock.GetLua(), functionName.c_str()); - call.PushString(jobId_); + call.PushString(event_.GetJobId()); call.Execute(); } } @@ -1056,20 +1045,9 @@ } - void LuaScripting::SignalJobSubmitted(const std::string& jobId) - { - pendingEvents_.Enqueue(new JobEvent(JobEvent::Type_Submitted, jobId)); - } - - - void LuaScripting::SignalJobSuccess(const std::string& jobId) + void LuaScripting::SignalJobEvent(const JobEvent& event) { - pendingEvents_.Enqueue(new JobEvent(JobEvent::Type_Success, jobId)); - } - - - void LuaScripting::SignalJobFailure(const std::string& jobId) - { - pendingEvents_.Enqueue(new JobEvent(JobEvent::Type_Failure, jobId)); + // Lua has its own event thread and queue to dissociate it completely from the main JobEventsThread + pendingEvents_.Enqueue(new LuaJobEvent(event)); } }
--- a/OrthancServer/Sources/LuaScripting.h Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancServer/Sources/LuaScripting.h Wed Nov 08 16:10:46 2023 +0100 @@ -24,6 +24,7 @@ #include "DicomInstanceToStore.h" #include "ServerIndexChange.h" +#include "JobEvent.h" #include "ServerJobs/LuaJobManager.h" #include "../../OrthancFramework/Sources/MultiThreading/SharedMessageQueue.h" @@ -47,7 +48,7 @@ class IEvent; class OnStoredInstanceEvent; class StableResourceEvent; - class JobEvent; + class LuaJobEvent; class DeleteEvent; class UpdateEvent; @@ -128,11 +129,7 @@ void Execute(const std::string& command); - void SignalJobSubmitted(const std::string& jobId); - - void SignalJobSuccess(const std::string& jobId); - - void SignalJobFailure(const std::string& jobId); + void SignalJobEvent(const JobEvent& event); TimeoutDicomConnectionManager& GetDicomConnectionManager() {
--- a/OrthancServer/Sources/ServerContext.cpp Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancServer/Sources/ServerContext.cpp Wed Nov 08 16:10:46 2023 +0100 @@ -164,7 +164,7 @@ } catch (...) { - throw OrthancException(ErrorCode_InternalError); + throw OrthancException(ErrorCode_InternalError, "Error while signaling a change"); } } catch (OrthancException& e) @@ -179,6 +179,48 @@ } + void ServerContext::JobEventsThread(ServerContext* that, + unsigned int sleepDelay) + { + while (!that->done_) + { + std::unique_ptr<IDynamicObject> obj(that->pendingJobEvents_.Dequeue(sleepDelay)); + + if (obj.get() != NULL) + { + const JobEvent& event = dynamic_cast<const JobEvent&>(*obj.get()); + + boost::shared_lock<boost::shared_mutex> lock(that->listenersMutex_); + for (ServerListeners::iterator it = that->listeners_.begin(); + it != that->listeners_.end(); ++it) + { + try + { + try + { + it->GetListener().SignalJobEvent(event); + } + catch (std::bad_alloc&) + { + LOG(ERROR) << "Not enough memory while signaling a job event"; + } + catch (...) + { + throw OrthancException(ErrorCode_InternalError, "Error while signaling a job event"); + } + } + catch (OrthancException& e) + { + LOG(ERROR) << "Error in the " << it->GetDescription() + << " callback while signaling a job event: " << e.What() + << " (code " << e.GetErrorCode() << ")"; + } + } + } + } + } + + void ServerContext::SaveJobsThread(ServerContext* that, unsigned int sleepDelay) { @@ -206,42 +248,21 @@ void ServerContext::SignalJobSubmitted(const std::string& jobId) { haveJobsChanged_ = true; - mainLua_.SignalJobSubmitted(jobId); - -#if ORTHANC_ENABLE_PLUGINS == 1 - if (HasPlugins()) - { - GetPlugins().SignalJobSubmitted(jobId); - } -#endif + pendingJobEvents_.Enqueue(new JobEvent(JobEventType_Submitted, jobId)); } void ServerContext::SignalJobSuccess(const std::string& jobId) { haveJobsChanged_ = true; - mainLua_.SignalJobSuccess(jobId); - -#if ORTHANC_ENABLE_PLUGINS == 1 - if (HasPlugins()) - { - GetPlugins().SignalJobSuccess(jobId); - } -#endif + pendingJobEvents_.Enqueue(new JobEvent(JobEventType_Success, jobId)); } void ServerContext::SignalJobFailure(const std::string& jobId) { haveJobsChanged_ = true; - mainLua_.SignalJobFailure(jobId); - -#if ORTHANC_ENABLE_PLUGINS == 1 - if (HasPlugins()) - { - GetPlugins().SignalJobFailure(jobId); - } -#endif + pendingJobEvents_.Enqueue(new JobEvent(JobEventType_Failure, jobId)); } @@ -449,6 +470,7 @@ listeners_.push_back(ServerListener(luaListener_, "Lua")); changeThread_ = boost::thread(ChangeThread, this, (unitTesting ? 20 : 100)); + jobEventsThread_ = boost::thread(JobEventsThread, this, (unitTesting ? 20 : 100)); #if HAVE_MALLOC_TRIM == 1 LOG(INFO) << "Starting memory trimming thread at 30 seconds interval"; @@ -494,6 +516,11 @@ changeThread_.join(); } + if (jobEventsThread_.joinable()) + { + jobEventsThread_.join(); + } + if (saveJobsThread_.joinable()) { saveJobsThread_.join();
--- a/OrthancServer/Sources/ServerContext.h Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancServer/Sources/ServerContext.h Wed Nov 08 16:10:46 2023 +0100 @@ -139,6 +139,11 @@ context_.mainLua_.SignalChange(change); } + virtual void SignalJobEvent(const JobEvent& event) ORTHANC_OVERRIDE + { + context_.mainLua_.SignalJobEvent(event); + } + virtual bool FilterIncomingInstance(const DicomInstanceToStore& instance, const Json::Value& simplified) ORTHANC_OVERRIDE { @@ -184,6 +189,9 @@ static void ChangeThread(ServerContext* that, unsigned int sleepDelay); + static void JobEventsThread(ServerContext* that, + unsigned int sleepDelay); + static void SaveJobsThread(ServerContext* that, unsigned int sleepDelay); @@ -233,7 +241,9 @@ bool haveJobsChanged_; bool isJobsEngineUnserialized_; SharedMessageQueue pendingChanges_; + SharedMessageQueue pendingJobEvents_; boost::thread changeThread_; + boost::thread jobEventsThread_; boost::thread saveJobsThread_; boost::thread memoryTrimmingThread_;
--- a/OrthancServer/Sources/main.cpp Wed Nov 08 09:59:31 2023 +0100 +++ b/OrthancServer/Sources/main.cpp Wed Nov 08 16:10:46 2023 +0100 @@ -1031,7 +1031,7 @@ httpServer.SetRemoteAccessAllowed(lock.GetConfiguration().GetBooleanParameter("RemoteAccessAllowed", false)); httpServer.SetKeepAliveEnabled(lock.GetConfiguration().GetBooleanParameter("KeepAlive", defaultKeepAlive)); httpServer.SetKeepAliveTimeout(lock.GetConfiguration().GetUnsignedIntegerParameter("KeepAliveTimeout", 1)); - httpServer.SetHttpCompressionEnabled(lock.GetConfiguration().GetBooleanParameter("HttpCompressionEnabled", true)); + httpServer.SetHttpCompressionEnabled(lock.GetConfiguration().GetBooleanParameter("HttpCompressionEnabled", false)); httpServer.SetTcpNoDelay(lock.GetConfiguration().GetBooleanParameter("TcpNoDelay", true)); httpServer.SetRequestTimeout(lock.GetConfiguration().GetUnsignedIntegerParameter("HttpRequestTimeout", 30)); @@ -1606,6 +1606,7 @@ lock.GetConfiguration().LoadModalitiesAndPeers(); } + // this function exits only when Orthanc stops or resets return ConfigureHttpHandler(context, plugins, loadJobsFromDatabase); } }
--- a/TODO Wed Nov 08 09:59:31 2023 +0100 +++ b/TODO Wed Nov 08 16:10:46 2023 +0100 @@ -54,6 +54,8 @@ - for job durations (+ have one histogram for each job) - for HTTP request handling - ... +* Investigate if one could fix KeepAlive race conditions: + https://discourse.orthanc-server.org/t/socket-hangup-with-rest-api/4023/3 ============================ Documentation (Orthanc Book) @@ -264,6 +266,9 @@ * Add a primitive for user authentication (to generate 401 HTTP status, whereas the "RegisterIncomingHttpRequestFilter()" can only generate 403 HTTP status) https://groups.google.com/g/orthanc-users/c/ymtaAmgSs6Q/m/PqVBactQAQAJ +* Add an index on the UUID column in the DelayedDeletion plugin: + https://discourse.orthanc-server.org/t/delayeddeletion-improvement-unique-index-on-pending-uuid-column/4032 + ---------------- Ideas of plugins