Modify disconnect method for proper socket shutdown

[ljwoods2]

Updated disconnect method to use SHUT_RDWR for proper socket closure.

Fixes #MDAnalysis/mdanalysis#5442

Changes made in this Pull Request:

• IMDServer.disconnect now properly sends an EOF to the client socket

PR Checklist

• Tests?
• Docs?
• CHANGELOG updated?
• Issue raised/referenced?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.77%. Comparing base (88f363c) to head (649f378).

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[orbeckst]

Is this change supposed to make it so that explicit server.disconnect() is not needed?

At least if that was the intention, it's not working for me. When I am trying this branch with the script below I need to use the disconnect, otherwise the loop only exits after timeout seconds.

import time

import imdclient
print(imdclient.__version__)

import MDAnalysis as mda; from MDAnalysis.tests import datafiles as data
import imdclient.tests.server, imdclient.tests.utils

# mda.start_logging()
mda.lib.log.create("imdclient", logfile="imdclient.log")


u = mda.Universe(data.TPR, data.TRR)
print(f"file = {data.TRR} with {u.atoms.n_atoms} atoms and {u.trajectory.n_frames} frames")

server = imdclient.tests.server.InThreadIMDServer(u.trajectory)
sinfo = imdclient.tests.utils.create_default_imdsinfo_v3()
sinfo.forces = False
server.set_imdsessioninfo(sinfo)
server.handshake_sequence("localhost", first_frame=False)
client = imdclient.IMDClient("localhost", port=server.port, n_atoms=u.atoms.n_atoms, timeout=5)

server.join_accept_thread()
server.send_frames(0, 5)
server.disconnect()  # needed so that client exits immediatedly and not waiting for timeout

i = 0
while True:
    try:
        t0 = time.perf_counter()
        frame = client.get_imdframe()
        elapsed = time.perf_counter() - t0
    except EOFError:
        elapsed = time.perf_counter() - t0
        print(f"EOFError after {elapsed:.3f}s: end of stream")
        break
    else:
        i += 1
        # Process and analyze the frame data as needed
        # For example, print frame number, simulation time, and positions of atom 0
        print(f"Frame {i}: time={frame.time}, atom 0 position={frame.positions[0]} ({elapsed:.3f}s)")

output from script when disconnect is NOT used, with timeout=5. Note that the last step takes 5 seconds.

(mdadev314) r2p2:imdclient oliver$ python run_server_test.py 
0.2.4+5.g649f378
file = /Users/oliver/Projects/mda/mdanalysis/testsuite/MDAnalysisTests/data/adk_oplsaa.trr with 47681 atoms and 10 frames
Frame 1: time=0.0, atom 0 position=[52.017067 43.56005  31.554958] (0.000s)
Frame 2: time=100.00000762939453, atom 0 position=[54.254    40.977192 29.265121] (0.000s)
Frame 3: time=200.00001525878906, atom 0 position=[54.564625 40.57454  29.494911] (0.000s)
Frame 4: time=300.0, atom 0 position=[55.90188  40.39242  27.285782] (0.000s)
Frame 5: time=400.0000305175781, atom 0 position=[54.90454  39.74054  27.653923] (0.000s)
EOFError after 5.003s: end of stream

With disconnect, it exits immediately.

[orbeckst]

I am reading your comment MDAnalysis/mdanalysis#5442 (comment) that this change to the shutdown flags is needed so that in all cases the disconnect properly ends the connection, even if reading and writing happens on the socket. Is that true?

If so, I am happy to merge this PR as is.

[ljwoods2]

This change is not supposed to make the server.disconnect() method unnecessary for quick shutdown, the timeout will still be fallen-back-on if disconnect is not called - it just makes the disconnect method "more correct".

The way I understand sockets, calling close() on them is not actually guaranteed to send an EOF to the peer if by some chance another process holds a file descriptor to the same server socket. We have no existing cases where this is true, which is why the MDA PR should stand on its own, but this PR just covers more cases if ever we were to have multiple processes hold a handle to the IMDServer socket

https://stackoverflow.com/questions/4160347/close-vs-shutdown-socket#comment4491371_4160356