search

lamyj / odil

Odil is a C++11 library for the DICOM standard
Other
85 stars 20 forks source link

Endless loop when a socket is closed without release #39

Open cguebert opened 7 years ago

cguebert commented 7 years ago

If a socket is closed on the other end without first sending an association release, my server is locked in a loop of : Operation error: End of file TCP timer started with pending operations TCP time out As I continue to try dispatching messages. We should detect this case and at least send an exception similar to AssociationAborted.

lamyj commented 7 years ago

As you mentioned in 514c8b6, that part of Odil is clearly not robust enough; I'll have to delve in ASIO documentation to adapt the current design. I'm not yet sure how to reconcile the OSI/state machine view of DICOM with the event-based design of Boost.asio, so any suggestion is welcome :smile:

By the way, sorry for the long delay in my answer: between vacations and being swamped in other projects, I haven't been able to work on Odil as much as I wanted to. Things should however be back to normal now.

cguebert commented 7 years ago

I think I am going to look into the network aspect of Odil in the near future. We can talk about it again in a few weeks, I will have read some ASIO documentation then.

lamyj commented 7 years ago

That would be great news! For future discussions, here are a few notes I took on this subject.

Impossibility to use one of Boost's state machine classes (Meta State Machine or State Chart): the DICOM one is too large and would need to change the hard-coded default values and recompile Boost. This won't play nice with the various packaging system, so I didn't go this way.

The state machine formalism (with a separation between states, transitions, and actions) is not perfect (at least when using ASIO) since a transition may occur in another transition: for example sending a PDU when the remote side has closed the socket will trigger an error transition from inside the sending-PDU transition. I think dropping the state/transition/action table and hard-coding the transitions in the actions would simplify the code.

The TCP/IP layer and the DUL layer must be more independent: not only is the error propagation of errors from TCP to DUL a bit of a mess, but the current design makes it impossible to fork just after receiving a TCP connection and thus hampers the usual implementation of a multi-threaded server (not sure how this applies on Windows).

There is a mechanism in ASIO to display the asynchronous operations as they are called: http://www.boost.org/doc/libs/1_62_0/doc/html/boost_asio/overview/core/handler_tracking.html