rbeckman-nextgen / test-mc

test-migration
1 stars 0 forks source link

Stopping / redeploying a channel cause error when source as "keep connection open" activated #3255

Open rbeckman-nextgen opened 5 years ago

rbeckman-nextgen commented 5 years ago

When we stop or redeploy a channel having a source connection opened, an exception is thrown. This happen when de "keep connection open" in the source is checked and the connection is currently opened. The connector type is "TCP Listener".

[2014-06-02 13:14:13,646] ERROR (com.mirth.connect.connectors.tcp.TcpReceiver:677): Error receiving message (TCP Listener "Source" on channel 726dfd71-61d8-40a3-b889-29fd374e0146). java.net.SocketException: socket closed at java.net.SocketInputStream.socketRead0(Native Method) at java.net.SocketInputStream.read(Unknown Source) at java.net.SocketInputStream.read(Unknown Source) at java.io.BufferedInputStream.fill(Unknown Source) at java.io.BufferedInputStream.read(Unknown Source) at com.mirth.connect.model.transmission.framemode.FrameStreamHandler.read(FrameStreamHandler.java:120) at com.mirth.connect.connectors.tcp.TcpReceiver$TcpReader.call(TcpReceiver.java:560) at com.mirth.connect.connectors.tcp.TcpReceiver$TcpReader.call(TcpReceiver.java:462) at java.util.concurrent.FutureTask.run(Unknown Source) at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) at java.lang.Thread.run(Unknown Source)

Imported Issue. Original Details: Reporter: zidore Created: 2014-06-02T10:21:59.000-0700

rbeckman-nextgen commented 5 years ago

When a receiver thread is in the middle of reading from the client socket and the channel is stopped, that socket is purposefully closed. This is because when the connection is kept open, that thread will be blocked indefinitely otherwise (at least, with the default receive timeout settings).

When an exception is caught during the read, we could check to see if the disposing boolean is true, and not log an error (maybe only warn). Of course that may mask legitimate errors you may want to see too. For example if a receiver thread is in the middle of reading and it has actually read some bytes from the client (like it's in the middle of processing a message), and you close the socket, then in that case you may want to know that you've actually prevented a message from being dispatched to the channel.

Imported Comment. Original Details: Author: narupley Created: 2014-06-02T10:33:09.000-0700

rbeckman-nextgen commented 5 years ago

It is not possible to close "gracefully" the opened connection? In the present case, we are stopping a channel intentionnaly so i expect to close the channel only between messages, not suddently like an "halt" button. In this situation, i think that an exception should not be thrown.

The stop channel should stop the socket correctly , i think

Imported Comment. Original Details: Author: zidore Created: 2014-06-02T10:44:38.000-0700

rbeckman-nextgen commented 5 years ago

Thing is, it is in between messages already. What I mean is, when a channel is stopped and a message is currently processing through it, it will wait until that message finishes before actually stopping the channel. However, what we define as "processing through a channel" is when a source connector calls dispatchRawMessage. The messaging engine has no idea that a source connector may be in the middle of reading a message from a socket (but isn't yet ready to call dispatchRawMessage) or something like that.

We might be able to expose a haveBytesBeenRead() method on StreamHandler so that the source connector (TcpReceiver) can detect whether it's currently reading an actual payload, or whether it's just idling on the blocking read(). In that case, we would be able to only close the socket if it's actually idling, and not otherwise. Though in that case you may get into a situation where the channel won't stop. If bytes have been read already but there's some connectivity problem or something causing the rest of the message to not come over the wire, and the listener has a receive timeout of zero (wait indefinitely), then the channel will remain in the Stopping state indefinitely, requiring a halt. But perhaps that's the correct behavior in this case.

Imported Comment. Original Details: Author: narupley Created: 2014-06-02T10:59:14.000-0700

rbeckman-nextgen commented 5 years ago

I have also run into this issue. Nick, I agree with your second paragraph. Stopping a channel should also wait for a half-received message to finish and there should be no error logged if in between message receipt. Having to halt the channel if some bytes have come seems fine to me.

The current situation effectively already masks legitimate errors because the same alert is sent whether in-between messages or halfway through receipt. So we have no way of knowing whether it's truly an erroneous condition as far as I can tell.

Imported Comment. Original Details: Author: dgtombs Created: 2014-08-28T10:23:38.000-0700

rbeckman-nextgen commented 5 years ago

While doing some testing with the HTTP Listener I found some related behavior worth nothing. I think some of the assumptions we made here are a little off. Biggest thing is that it is possible for us to call stop on a source connector after dispatchRawMessage but before finishDispatch is called. I don't think that in these cases we would see a "socket closed" error but we just don't respond on a socket we closed. The chances of this happening are pretty slim but it is possible.

Inside dispatchRawMessage we check shuttingDown to make sure that the channel is not shutting down. The thing is that stopping a channel doesn't actually set shuttingDown until after the source connector has been stopped(there is good reason for this). This means that we could be calling stop after accepting a message but before responding to the client. In the case of the HTTP Listener we would actually stop the jetty server before it had a chance to respond to the client. On the HTTP Sender side you would see something like "server failed to respond." To reproduce this I had a pretty unrealistic setup(alot of threads and constantly restarting channel) but it is a possibility.

This is separate from the case where the source connector is in the middle of reading a message. In that case dispatchRawMessage is never called.

Imported Comment. Original Details: Author: eduardoa Created: 2016-09-13T17:56:06.000-0700