marcelvanherk / Conquest-DICOM-Server

Conquest DICOM server, aiming for complete source code release
112 stars 47 forks source link

Association.ConnectedIP unreliable #29

Closed msz-kp closed 1 year ago

msz-kp commented 1 year ago

Hi! I noticed that Association.ConnectedIP may be incorrect in case of simultaneous connections. Everything seems ok when dgate is run in single thread mode(with -b parameter) some kind of race condition ? (tested on current git and 1.4.19d1)

scenario: connect local dgate server to ethernet interface(192.168.1.1) or loopback

dicom.ini:

[lua] Association = dofile('./lua/association.lua'); ImportConverter0 = dofile('./lua/import.lua');

association.lua:

AssocIp = Association.ConnectedIP
AssocAet = Association.Calling

if (Association.ConnectedIP == '192.168.1.1') then
   if not (Association.Calling == 'CLI192')
   then
      print( '192.168.1.1 ERRDBG(association): ', Association.Calling, Association.Called, Association.Thread, Association.ConnectedIP )
   end
elseif (Association.ConnectedIP == '127.0.0.1') then
   if not Association.Calling == 'CLI127'
   then
      print('127.0.0.1 ERRDBG(association): ', Association.Calling, Association.Called, Association.Thread, Association.ConnectedIP)
   end
end

import.lua:

if (AssocIp == '192.168.1.1') then
   if not (Data.StudyInstanceUID == '1.3.12.2.1107.5.4.3.11540117440512.19970422.140030.45')
   then
      print('192.168.1.1 ERRDBG(import/ip): ', Association.Calling, Association.Called, Association.Thread, Association.ConnectedIP, AssocIp, Data.StudyInstanceUID)
   end
elseif (AssocIp == '127.0.0.1') then
   if not (Data.StudyInstanceUID == '1.3.46.670589.11.0.1.1996082307380006')
   then
      print('127.0.0.1 ERRDBG(import/ip): ', Association.Calling, Association.Called, Association.Thread, Association.ConnectedIP, AssocIp , Data.StudyInstanceUID)
   end
end

-- this test never fire up, AssocAet == Association.Calling
if (AssocAet == 'CLI192') then
   if not (Data.StudyInstanceUID == '1.3.12.2.1107.5.4.3.11540117440512.19970422.140030.45')
   then
      print('CLI192 ERRDBG(import/aet): ', Association.Calling, Association.Called, Association.Thread, Association.ConnectedIP, AssocIp, Data.StudyInstanceUID)
   end
elseif (AssocAet == 'CLI127') then
   if not (Data.StudyInstanceUID == '1.3.46.670589.11.0.1.1996082307380006')
   then
      print('CLI127 ERRDBG(import/aet): ', Association.Calling, Association.Called, Association.Thread, Association.ConnectedIP, AssocIp,Data.StudyInstanceUID)
   end
end

simple code in ruby to test scenario (require "dicom" gem)

require 'dicom'
include DICOM

def send1
  4.times do
    fork do
      # study with uid: 1.3.12.2.1107.5.4.3.11540117440512.19970422.140030.45
      node = DClient.new('192.168.1.1', 11112, ae: 'CLI192', host_ae: 'PACS')
      node.send('1.dcm')
    end
  end
end

def send2
  4.times do
    fork do
      # study with uid: 1.3.46.670589.11.0.1.1996082307380006
      node = DClient.new('127.0.0.1', 11112, ae: 'CLI127', host_ae: 'PACS')
      node.send('2.dcm')
    end
  end
end

10.times do
  fork { send1 }
  fork { send2 }
end

results in: 192.168.1.1 ERRDBG(association): CLI127 PACS 5 192.168.1.1 192.168.1.1 ERRDBG(import/ip): CLI127 PACS 5 0.0.0.0 192.168.1.1 1.3.46.670589.11.0.1.1996082307380006 127.0.0.1 ERRDBG(import/ip): CLI192 PACS 1 0.0.0.0 127.0.0.1 1.3.12.2.1107.5.4.3.11540117440512.19970422.140030.45 127.0.0.1 ERRDBG(import/ip): CLI192 PACS 3 0.0.0.0 127.0.0.1 1.3.12.2.1107.5.4.3.11540117440512.19970422.140030.45 192.168.1.1 ERRDBG(association): CLI127 PACS 36 192.168.1.1 192.168.1.1 ERRDBG(import/ip): CLI127 PACS 36 0.0.0.0 192.168.1.1 1.3.46.670589.11.0.1.1996082307380006

full log: output.log

marcelvanherk commented 1 year ago

dgate64.zip

Can you try this one? I moved reading connectedIP above the while(Lock) loop.

Marcel

msz-kp commented 1 year ago

Unfortunately nothing has changed

msz-kp commented 1 year ago

isnt it that?: instance of DriverApp class is responsible for handle incoming connection on connect it:

because if i understand it correctly, next connection going to overwrite ConnectedIP which is passed to lua later

i have implemented ConnectedIP as thread local storage variable(which is set before DriverHelper release a lock) and it seems to solve that problem

not sending pull request because im not sure its a right approach, will not introduce another problem and it is only pthread implementation not compatible with win32 threads actually it is just one big experiment but maybe its a good point to start at code at: https://github.com/msz-kp/Conquest-DICOM-Server

marcelvanherk commented 1 year ago

Hi,

I fixed it (I think) in 1.5.0d. Can you test?

msz-kp commented 1 year ago

hi everything seems to be all right now (checked only on association level and there is no mismatch between aet and ip address)