[SOLVED] Problem with two SCPs in a single process

All other questions regarding DCMTK

Moderator: Moderator Team

Post Reply
Message
Author
pgimeno
Posts: 9
Joined: Tue, 2023-02-07, 08:24

[SOLVED] Problem with two SCPs in a single process

#1 Post by pgimeno »

In my application I have implemented a thread with a C-MOVE SCU + C-STORE SCP based on the code from dcmnet/apps/movescu.cc, which is pretty much what I needed for my application. I can process the input files (which are structured reports, therefore I'm using a DSRDocument to process them) as they are transmitted without even needing to store them on disk; the data is correctly processed without problems. The SCP part listens on port 11112 and the PACS transmits the data without problems.

Then I need another C-STORE SCP, in a different port (5678), with a different AE. So I have set up a second thread for that purpose; at the moment it's something very simple (and it won't get much more complex than that, just some error handling and configurability):

Code: Select all

    storscp = new DcmStorageSCP();

    storscp->loadAssociationConfiguration("/etc/dcmtk/storescp.cfg",
        "Default");
    storscp->setOutputDirectory("/some/dir");
    storscp->setAETitle("AUXSCP");
    storscp->setPort(5678);

    // does not return
    storscp->listen();
But to my surprise, when I start this second thread, the first thread stops working properly. The invocation of sr->read(dataset, DSRTypes::RF_acceptInvalidContentItemValue), which worked properly when it was not running, fails now with an "Invalid Value" condition, and that same call issues the following warning:

Code: Select all

W: SOPClassUID (0008,0016) violates VR definition in SOPCommonModule
The DSRDocument is of course not populated. Note that by that time, the second thread has not established any association whatsoever; it's merely listening. This is test data; all the time the first thread is requesting and processing the very same file.

If I only comment out the storscp->listen() call and replace it with an infinite sleep loop, DSRDocument::read() works fine again, and the file is correctly processed.

This is 100% reproducible; the read() call only works when the listen() call is not executed, and it only fails when the listen() call is executing.

According to this FAQ entry: https://forum.dcmtk.org/viewtopic.php?t=24 DCMTK is thread-safe starting with version 3.6.2. I'm using version 3.6.5 as packaged by Debian.

What may be going on here?
Last edited by pgimeno on Wed, 2023-03-15, 08:34, edited 1 time in total.

J. Riesmeier
DCMTK Developer
Posts: 2501
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: Problem with two SCPs in a single process

#2 Post by J. Riesmeier »

My first idea was: Could you print the value of SOPClassUID stored in "dataset" or the entire "dataset" before and after calling "sr->read()"?

But then, the following came to my mind: Could you check what the value of dcmEnableAutomaticInputDataCorrection.get() is? Maybe, your problem is related to an issue with 0-byte padded UID values. See this commit: https://git.dcmtk.org/?p=dcmtk.git;a=co ... 43f380d838
In my application I have implemented a thread with a C-MOVE SCU + C-STORE SCP based on the code from dcmnet/apps/movescu.cc
movescu is a command line tool and was never designed to be used in a multi-threaded environment. Is there a reason why you did not implement your C-MOVE SCU based on DcmSCU and your C-STORE SCP based on DcmStorageSCP (or DcmSCP)?

J. Riesmeier
DCMTK Developer
Posts: 2501
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: Problem with two SCPs in a single process

#3 Post by J. Riesmeier »

I've also found that DcmSCP does the following in DcmSCP::openListenPort():

Code: Select all

    // make sure not to let dcmdata remove trailing blank padding or perform other
    // manipulations. We want to see the real data.
    dcmEnableAutomaticInputDataCorrection.set(OFFalse);
I'll ask the original author of this class whether this is really needed...

Michael Onken
DCMTK Developer
Posts: 2048
Joined: Fri, 2004-11-05, 13:47
Location: Oldenburg, Germany
Contact:

Re: Problem with two SCPs in a single process

#4 Post by Michael Onken »

Being the original author:

I think in general it is a good idea to "store" the file without removing extra padding and so on in DcmSCP (i.e. dcmEnableAutomaticInputDataCorrection = false). However, since it is global flag we should not set it within the class but instead let the user decide outside DcmSCP. So, overall I think it is better to remove the line and not to set dcmEnableAutomaticInputDataCorrection within DcmSCP at all.

Is this also what you have in mind Jörg?

BR Michael

J. Riesmeier
DCMTK Developer
Posts: 2501
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: Problem with two SCPs in a single process

#5 Post by J. Riesmeier »

Changing global settings (e.g. OFGlobal instances) within a class is never a good idea, also in this case. Therefore, I would agree that removing the line(s) from DcmSCP would be the preferred solution.

DcmStorageSCP has a "bit preserving mode" (similar to the one of storescp), so you should document this new behavior "somewhere" (e.g. in the DcmSCP and/or DcmStorageSCP header). Furthermore, I would suggest to add "dcmEnableAutomaticInputDataCorrection.set(OFFalse)" to the bit preserving mode of dcmrecv (which is based on DcmStorageSCP).

Michael Onken
DCMTK Developer
Posts: 2048
Joined: Fri, 2004-11-05, 13:47
Location: Oldenburg, Germany
Contact:

Re: Problem with two SCPs in a single process

#6 Post by Michael Onken »

dcmEnableAutomaticInputDataCorrection.set(OFTrue)" to the bit preserving mode of dcmrecv (which is based on DcmStorageSCP).
You mean dcmEnableAutomaticInputDataCorrection.set(OFFalse)?

[Edit: OK, I see you fixed it already in the post]

pgimeno
Posts: 9
Joined: Tue, 2023-02-07, 08:24

Re: Problem with two SCPs in a single process

#7 Post by pgimeno »

J. Riesmeier wrote: Tue, 2023-03-14, 17:32 But then, the following came to my mind: Could you check what the value of dcmEnableAutomaticInputDataCorrection.get() is? Maybe, your problem is related to an issue with 0-byte padded UID values. See this commit: https://git.dcmtk.org/?p=dcmtk.git;a=co ... 43f380d838
Spot-on! Indeed it was true all the time when the second thread was stopped, and when it ran, it was set to false by the time the file was being read. Indeed the SOP class UID had an odd length, a `dcmdump --no-uid-names --disable-correction` confirmed that it was NULL-padded, and enabling the setting after ensuring that the second thread was listening allowed the file in the first thread to be correctly processed. Thank you very much!

I can't help wondering why this is a global setting in the first place, though, rather than something like a flag in DcmDocument DcmDataset or similar that can be independently set for different contexts.
J. Riesmeier wrote: Tue, 2023-03-14, 17:32 movescu is a command line tool and was never designed to be used in a multi-threaded environment. Is there a reason why you did not implement your C-MOVE SCU based on DcmSCU and your C-STORE SCP based on DcmStorageSCP (or DcmSCP)?
Yes, it's discussed in this thread: https://forum.dcmtk.org/viewtopic.php?f=1&t=5237 - basically, the unstoppability of the server, the possibility of race conditions and the wish to keep the connection closed until necessary for the transmission. The difficulty of stopping the listening loop is a design problem in my opinion.

As for being designed as a command line tool, I had to plug some leaks that in a context of a process designed to run and terminate don't matter, but in the context of a continuous loop within an application do.
Last edited by pgimeno on Wed, 2023-03-15, 09:38, edited 2 times in total.

J. Riesmeier
DCMTK Developer
Posts: 2501
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: [SOLVED] Problem with two SCPs in a single process

#8 Post by J. Riesmeier »

I can't help wondering why this is a global setting in the first place, though, rather than something like a flag in DcmDocument or similar that can be independently set for different contexts.
You mean DcmDataset? This is mainly for historical reasons. When the DICOM dataset parser in "dcmdata" was designed mid of the 90s, apparently nobody thought about all the switches and workarounds that would be needed in practise. Also at that time, DCMTK was only used on Unix system, where "fork" was the standard way to create multiple processes (instead of threads, which is more common nowadays). In fact, dcmEnableAutomaticInputDataCorrection, which is an instance of the OFGlobal<> class, is already the thread-safe variant of a global variable. We know that this is not perfect, but changing the parser completely would be a major undertaking.
Indeed the UID had an odd length, a `dcmdump --no-uid-names --disable-correction` confirmed that it was NULL-padded [...]
By the way, using dcmdump with option --quote-as-octal (+Qo) will actually show the 0-byte at the end of the UID values.

pgimeno
Posts: 9
Joined: Tue, 2023-02-07, 08:24

Re: [SOLVED] Problem with two SCPs in a single process

#9 Post by pgimeno »

I mixed DSRDocument and DcmDataset, sorry.

Browsing the result with less exposed the NULL as an inverse-colour ^@ which was enough.

One question, what do you mean by "is already the thread-safe variant of a global variable"? It's definitely not thread-local, at least not in version 3.6.5, as there was a thread influencing another.

J. Riesmeier
DCMTK Developer
Posts: 2501
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: [SOLVED] Problem with two SCPs in a single process

#10 Post by J. Riesmeier »

One question, what do you mean by "is already the thread-safe variant of a global variable"? It's definitely not thread-local, at least not in version 3.6.5, as there was a thread influencing another.
Thread safety does not always mean thread-local storage. In this case, it's a global variable where the OFGlobal<> class makes sure that the access to this variable is serialized, i.e. only one thread reads or writes it at a time. The implementation is based on a mutex if the DCMTK is compiled with multi-thread support.

Post Reply

Who is online

Users browsing this forum: Ahrefs [Bot], Bing [Bot], Google [Bot] and 1 guest