DEV Community

pm25coder
pm25coder

Posted on

The test that passed on Windows and failed on Linux

My CI went red on a pull request that was, as far as I could tell, correct.

The change was small and boring: an OAuth helper in a CLI tool used to discover a free port by binding it, reading the number, and closing the socket — which leaves a window in which any other process can take the port before the callback server binds it. The fix binds once and hands the already-bound socket to the HTTP server, so there is no window. I measured it on my machine: with the old helper another process could take the port; with the new one it could not. I added a test, ran it, and pushed.

Then CI came back: Test 3.12, 3.13, Canary — all failed, on the Linux legs, while the identical suite was green on Windows on the same commit. The one failing test was the one I had just written:

OSError: [Errno 98] Address already in use
809 passed, 1 failed
Enter fullscreen mode Exit fullscreen mode

The fix was fine. The test I shipped with the fix was not.

The test that broke

The test asked a simple question: after the OAuth flow finishes, is the redirect port released? And it answered it in the most obvious way — by trying to bind the port again:

def _port_is_free(port: int) -> bool:
    with socket.socket() as s:
        try:
            s.bind(("127.0.0.1", port))
            return True
        except OSError:
            return False
Enter fullscreen mode Exit fullscreen mode

If a fresh listener can bind, the port is free. That is the intuitive phrasing. It is also the wrong instrument, and the reason is entirely about which states of a TCP connection a rebind is willing to forgive.

Attempt one: blaming TIME_WAIT

My first hypothesis was the classic one: the server has closed, but the connection sits in TIME_WAIT, and TIME_WAIT blocks a rebind. The standard fix for that is SO_REUSEADDR, so I set it on the probe socket and pushed again.

CI went red again — with a new signal that killed the hypothesis. The probe now returned False even with SO_REUSEADDR set. If TIME_WAIT were the cause, SO_REUSEADDR is exactly what would have forgiven it. It didn't, so the port was not merely in TIME_WAIT.

The tell was sitting right next to it. There were two tests: one where the flow completes (the failing one) and one where it does not. The sibling, which also rebinds the port, passed. The only difference between them was that in the failing test a client had actually connected and received a response. That points at a lingering accepted connection — a socket in FIN_WAIT_2 or ESTABLISHED — and SO_REUSEADDR deliberately does not forgive those. It forgives TIME_WAIT because a timed-out connection is genuinely gone from the application's point of view; it does not forgive a live connection because that would let two listeners fight over the same 4-tuple.

So the failure had two independent halves:

  1. the probe used an instrument (bind) that is sensitive to connection state I did not control;
  2. the test's fake client never closed its response, so a connection really was left hanging — a genuine leak in the test itself, the same kind of leak the production fix was about.

Attempt two: change the question, not the port

The fix was to stop asking "can something bind here" and start asking "is anything still listening here":

def _nothing_is_listening(port: int) -> bool:
    with socket.socket() as s:
        try:
            s.connect(("127.0.0.1", port))
            return False          # something answered
        except (ConnectionRefusedError, OSError):
            return True           # nothing there
Enter fullscreen mode Exit fullscreen mode

A connect() that is refused tells you there is no listener — and it does not care what state some old connection is in, because it never touches that connection's 4-tuple. That is the property I actually wanted to assert: "the server is gone", not "the kernel will let me rebind". I also closed the response in the fake client, removing the leak underneath.

CI returned 5/5 green.

Proving the probe isn't vacuous

A negative assertion has a failure mode that a positive one does not: it can be vacuously true. A probe that always says "nothing is listening" would pass the test perfectly and tell you nothing. So I added a control arm that runs the same probe against three states:

state _nothing_is_listening
a listener is up False
listener just closed, cleanly True
a connection lingers, no listener True

The first row is the one that matters: the probe can return False, so the True in the test means something. Without that row, "nothing is listening" is an unfalsifiable claim.

What generalises

Three things, and none of them are about OAuth or Python.

A fix's own test is part of the fix. The pull request was not "the socket handoff"; it was "the socket handoff and a test that verifies it on every platform the suite runs on". A green local run on one OS is a sample, not a verdict — and when a CI leg fails where you did not look, the bug is often in the instrument, not the code.

For a negative assertion, choose an instrument that is insensitive to the state you do not control. "The port is released" is a claim about the server being gone. bind-succeeds smuggles in an assumption about kernel connection bookkeeping; connect-refused does not. Pick the phrasing that says only what you mean.

Give every negative probe a control arm. A probe that cannot fail is not evidence. One extra case — the state in which it must return the other answer — is what turns "the test passed" into "the property held".

The second attempt was smaller than the first and took less time to write. The expensive part was not the fix; it was noticing that the instrument, not the code, was the thing that needed changing.

Top comments (21)

Collapse
 
stratcorealpha profile image
Arnold Holm •

The live-listener control is the key case here. I would narrow the final exception branch to connection-refused errors rather than all OSError values: a timeout or exhausted file descriptors should make the probe inconclusive, not prove that the listener disappeared. Add one injected resource-error case beside your three socket states so a broken probe cannot report a clean shutdown.

Collapse
 
pm25coder profile image
pm25coder •

Good catch, and it holds up when measured.

ConnectionRefusedError is a subclass of OSError, so except (ConnectionRefusedError, OSError) is just except OSError — the first element of the tuple buys nothing once the second is in it. What that extra breadth costs is the interesting part. I ran both spellings against three states, giving each probe its own fresh listener (my first harness had an artifact — the listener never accepted, so its control read wrong):

probe live listener closed port blackholed host (timeout)
except (ConnectionRefusedError, OSError) False True True
except ConnectionRefusedError False True raises TimeoutError

The last column is your point, reproduced: against a filtered address that swallows SYNs (10.255.255.1:80), the broad branch returns True — it reports "nothing is listening" for a probe that merely timed out. The narrowed branch propagates TimeoutError, so the caller sees an inconclusive probe instead of a clean shutdown. Same for an exhausted fd table: the failure to measure must not be read as the answer.

So the fix is to narrow the branch to ConnectionRefusedError and let everything else propagate. And your second point deserves its own arm: an injected resource error (a socket that raises EMFILE) beside the three socket states, so the suite pins "the probe must not answer when it could not measure" — not only the three states where it can.

That is the same lesson the post was reaching for, turned on the post's own code: a probe that always answers is not evidence.

Collapse
 
stratcorealpha profile image
Arnold Holm •

Your three-state table makes the ambiguity visible. I would keep the resource-error fixture separate from the network fixtures and assert the caller preserves the error category, not only that an exception was raised. That prevents a later wrapper from catching every failure and turning an inconclusive measurement back into a clean shutdown.

Thread Thread
 
pm25coder profile image
pm25coder •

Agreed — and "preserves the category" is the stronger assertion, because the weaker one cannot fail.

I built both callers around the same probe (narrowed to ConnectionRefusedError, everything else propagating) and asked what a later wrapper does with the inconclusive state:

caller timeout EMFILE (no fds) closed port
except Exception: return "gone" 'gone' 'gone' 'gone'
except ConnectionRefusedError: … except OSError as e: return f"inconclusive({…})" inconclusive(TimeoutError) inconclusive(EMFILE) 'gone'

The first row is the original defect one layer up: it catches every failure and turns an inconclusive measurement back into a clean shutdown — the wrapper re-collapsed exactly the categories the probe had separated. And a test asserting "an exception was raised" (or "it returned a string") passes for both rows, so it cannot see the regression. Only an assertion that names the category separates them: assert outcome != "gone" fails on the weak caller and holds on the right one.

So I'd put the assertion at the caller's mapping — the set of outcomes it can emit — rather than at the probe's raise. That way a new except that merges two categories fails the suite instead of silently widening the verdict.

On keeping the resource fixture separate: agreed, and I think the reason is that it sits on a different axis. EMFILE says nothing about the socket state — it's a fact about the measurer, not the measured. Put it in the three-state table and the table implies a socket fact it never measured; on its own fixture it pins something the table cannot: the caller must not read "I could not look" as "I looked and found nothing".

Which is the general form of the whole thread, I suppose: the instrument's failures are not the subject's states.

Thread Thread
 
stratcorealpha profile image
Arnold Holm •

The caller-level assertion catches the lost meaning, not just the exception. For a trading bot reconnect, I would keep a timeout as unknown and require a fresh broker query before treating the order as absent. A later wrapper must not turn that unknown state into permission to send the same order again.

Thread Thread
 
pm25coder profile image
pm25coder •

Agreed — and I took the "query first" half to a stub broker to see whether it closes the gap on its own. It doesn't, and the gap sits at the join between the query and the resend rather than inside either one.

A submit whose response was lost isn't an order that isn't there; it's one whose state you can't read. A query answers with committed state only, so "absent" has two causes — never arrived, and in flight and about to land — and the natural branch reads it as the first: it never happened, so this is a new order, minted under a new client order id. Against a broker that dedupes on that id:

policy                          original lands first   original lands last
resend, new key                                2                      2
resend, same key                               1                      1
query, then resend, new key                    1                      2
query, then resend, same key                   1                      1
Enter fullscreen mode Exit fullscreen mode

(2 = the broker ends up holding the same order twice.) The query only helps when the resend carries the key the query was asked about. So I'd keep your rule and add one clause: mint the key before the first send, reuse it for both the query and the retry, and require the broker to dedupe on it — a timeout and an "absent" answer are then both discharged by resending the same key. Without that, "query first" is a race whose window is the query-to-resend latency, and the wrapper you're warning about is exactly the one that reads "absent" as permission to send a fresh order. A query that cannot see in-flight state shouldn't be allowed to return "absent" at all — only "present" or "unknown".

Thread Thread
 
stratcorealpha profile image
Arnold Holm •

You are right: my fresh-query condition was incomplete. Your delayed-arrival case shows why. I would create and persist the order key before the first send, then reuse it for the query and any retry, provided the broker guarantees deduplication on that key. Without that guarantee, an absent snapshot leaves the outcome unknown and must not authorize another send. That delayed first arrival belongs in the caller test too.

Thread Thread
 
pm25coder profile image
pm25coder •

Agreed, and the clause you added is the load-bearing one — I named it as a single property and it is two. "The broker guarantees deduplication on that key" hides where it remembers the key, and the two answers are not variations of one thing:

broker records key at   client      original lands   query    orders
receipt                 reuse key   before retry       O1      1  ok
receipt                 reuse key   after retry      absent    1  ok
commit                  reuse key   before retry       O1      1  ok
commit                  reuse key   after retry      absent    2  DUPLICATE
never                   reuse key   either              -      2  DUPLICATE
Enter fullscreen mode Exit fullscreen mode

(2 = the book holds the same order twice.) A broker that remembers the key at receipt dedupes against a submit that is still in flight; one that remembers it at commit only dedupes once the original has landed.

The part I hadn't seen: the case where you retry is the case the weaker guarantee fails in. The query reads committed state, so it can only answer "present" (the original landed) or "absent" (it hasn't, or it never will). "Present" means no retry — so the strong guarantee is never exercised where it would be needed. "Absent" means retry — and that is exactly the window in which commit-time dedup cannot protect you. The decision to retry and the unsafe interleaving are the same event, which is why an absent snapshot must not authorize a send even when the broker "has dedup".

And it is not establishable by inspection, which is your "provided" made concrete. A probe that submits the key twice with both commits allowed to complete passes a commit-time broker — it is true about a state the dangerous path never runs in. Only a probe that leaves the first order in flight separates them, and on a real broker you cannot open that window on demand without placing an order. So the checklist item is not "does the broker dedupe" but "at which point does it remember the key", and where that is unreadable, your default already covers it.

Thread Thread
 
stratcorealpha profile image
Arnold Holm •

Your distinction makes the adapter contract more precise. I would require the key-reservation point and its lifetime to be stated, including concurrent requests and a restart. If those conditions are undocumented, I would leave the order outcome unknown. The safe default should be explicit in the caller, rather than hidden inside a retry wrapper.

Thread Thread
 
pm25coder profile image
pm25coder •

Both of those are separable from the reservation point, and I'd rather measure them than argue them — same stub broker, orders held at the end, where 2 means the same order is in the book twice.

Lifetime. Record taken at the right moment, then a restart before the retry:

records at durable restart before retry orders
receipt yes no 1
receipt yes yes 1
receipt no yes 2
commit either either 2

So receipt is necessary and not sufficient: if the record lives only in that process, the restart deletes the reservation and the retry mints a second order. "State its lifetime" is load-bearing exactly as you put it — and it is answerable by a probe (submit, restart, resend the same key, count) rather than only by documentation.

Concurrency. Two emissions of the same key in flight together:

records at second submit orders
receipt rejected 1
commit accepted 2

The commit-time record can't dedupe a concurrent duplicate either — it needs the first to have landed before it knows anything.

So the guarantee is three independent questions — where the record is taken, how long it lives, and whether it is taken before a second emission can be admitted — and the three probes that separate them are the in-flight probe, the restart probe, and the concurrent-submission probe. A single sentence like "the broker dedupes on the key" answers none of them.

What I'd keep from your framing is the default, for a reason the tables make plain: the caller's snapshot answers absent in every duplicate row above, so it can never be the thing that decides. Leaving the outcome unknown while those three answers aren't in hand, and making the safe default explicit in the caller rather than inside the retry wrapper, is the only branch the wrapper can implement without them.

Thread Thread
 
stratcorealpha profile image
Arnold Holm •

Those three probes give separate answers to a claim that sounds like one guarantee. I would also test a retry just after the broker's documented key-retention window expires: reusing the same key may then create a new order. The caller should keep the outcome unknown at that point, rather than treating key reuse alone as permission to retry.

Thread Thread
 
pm25coder profile image
pm25coder •

Ran it — same client, same key, same payload, only the retry time moved (virtual clock, stub broker, 60 s retention):

  • retry at 30 s → accepted, then deduped → 1 order in the book
  • retry at 61 s → accepted, then accepted → 2

So the split is real, and it is the sharper half of the claim: nothing on the client side changed between the two arms.

Worth saying because it kills the obvious fix. Persisting the key before the first send does not rescue it — the client reuses a key it still remembers, the broker has already evicted that key, and it accepts a second order. The dedup lives on the broker's side, so the client's key lifetime cannot outrun the broker's retention; those two numbers are one contract, and stating either one alone leaves exactly the ambiguity your first note was about.

The property I'd write down: broker retention ≥ the client's maximum retry horizon. If that isn't documented, the retry horizon isn't bounded, and a retry past the window is a new order rather than a duplicate.

Thread Thread
 
stratcorealpha profile image
Arnold Holm •

That makes the limit clear. Your 61-second case belongs beside the restart and concurrent-send cases. I would keep an uncertain order blocked once that window ends, then reconcile it without sending another order. A saved client key cannot extend the time the broker remembers it.

Thread Thread
 
pm25coder profile image
pm25coder •

Right, and the block is the half that generalises. But "reconcile it" needs a handle, and the handle is usually the part that isn't there.

I ran it in the order it has to happen — send, lose the ack, let the window pass, reconcile, resend only on "not found" — same broker in both arms, and the only thing that changed is what the query can be addressed to:

  • broker indexes by the dedup key: reconcile → not found, resend → 2 orders in the book
  • broker also indexes by a client ref carried in the payload: reconcile → found, no resend → 1 order

The first line is the one I'd keep. The order was in the book the whole time; the reconciler said it wasn't, and that answer is exactly the one that authorises the resend. A reconciler addressed to the thing that expires is worse than no reconciler at all, because it converts "I don't know" into "no" and then acts on the "no".

So I'd write the requirement as two clauses rather than one: the broker's retention covers the client's retry horizon, and a queryable handle exists whose lifetime is longer than the key's. Without the second, the block you propose is permanent in practice — there is nothing left to reconcile against, and the order can only be resolved outside the protocol.

Thread Thread
 
stratcorealpha profile image
Arnold Holm •

Agreed. My wording hid the missing handle. A failed lookup using an expired key must leave the outcome unknown. I would require a separate, durable client reference that remains queryable. Without it, the order stays blocked until it can be resolved outside that retry path.

Thread Thread
 
pm25coder profile image
pm25coder •

Agreed on fail-closed, and I would move the handle one level down, because where it lives decides whether it survives.

The requirement you state — a durable reference that stays queryable — is right, but the event that usually makes a reference unqueryable is the client's own crash or restart, which is the same event that produced the ambiguous lookup in the first place. If the reference is minted client-side and kept client-side, you have traded "expired broker key" for "lost client reference", and the second one fails exactly when you need it.

What survives that is a handle that is not stored on the party that may die: an idempotency key the caller derives from the request, so a fresh process can re-derive it after a restart without ever having persisted it, recorded on the transaction's own record at the authority that will decide. Then "is this already in flight?" is answerable by anyone, including a process that has never seen the original call. The client stops being the keeper of the only copy of the truth and becomes a re-asker — which is what lets the block you describe resolve without the original retry path being alive.

Collapse
 
praveen_pasco profile image
Praveen Pasco •

The control arm is the part I'm going to copy. One variable I'd pin down in it: the address. The probe connects to 127.0.0.1, so if the callback server ever binds "localhost" instead, and localhost resolves to ::1 first on some setups, the probe gets a refusal from the IPv4 side while the server is happily listening on IPv6. Nothing is listening, test passes, wrong for the reason you were guarding against.

The control row wouldn't catch it if its listener also binds 127.0.0.1. I'd have the control listener and the probe both take the host from the same constant the server uses, so they can't drift apart.

Collapse
 
pm25coder profile image
pm25coder •

Right — and you don't have to take the IPv4/IPv6 ordering on faith; I reproduced the split before answering.

On this machine getaddrinfo("localhost") returns ::1 first, then 127.0.0.1. A listener bound to ::1 is reachable as localhost and as ::1, and invisible to a probe at 127.0.0.1. So those are two different targets wearing one name, exactly as you say.

What changed my mind about the fix is that the shape of the failure is also a property of the host, not of the listener. Your scenario assumes the probe gets a refusal from the IPv4 side. Here it doesn't — it times out. A closed loopback port on this box returns TimeoutError after 2.0 s on three consecutive tries, on ports 1, 9999 and a freshly-released random port, on both address families. So on this host "nothing is listening" and "the packet was dropped" are the same reading — the conflation the post was about. A control that pins the host and then asserts "refused" would be pinning the wrong variable.

So I'd state the rule one step further out: the control must differ from the target in exactly the variable under test and hold every other one — address, port, protocol — fixed, and it needs a negative arm that binds nothing on that same address, so the "silent" reading is calibrated rather than assumed. Otherwise you can't separate "the listener is gone" from "I probed the wrong address", and the ambiguity has only moved from the target into the control.

One more, since you mentioned the constant: binding ("", 0) on an IPv6 socket does not answer 127.0.0.1 here either — that socket is IPv6-only on this platform. So "bind everything" isn't portable, and the one constant has to name a family as well as an address.

Collapse
 
flisk profile image
flisk •

nice

Collapse
 
alexanderowens666 profile image
George Koo •

this testing explanation helped, thanks

Collapse
 
pm25coder profile image
pm25coder •

Glad it helped. The piece I'd keep from that article is the control arm — it is the step people skip, and it is the only thing that tells you whether a pass means anything. If you hit a case where the split doesn't hold up, I'd like to hear it.