|
16 | 16 | """ |
17 | 17 |
|
18 | 18 | import pytest |
| 19 | +import os |
| 20 | +import re |
| 21 | +import subprocess |
| 22 | +import sys |
| 23 | +import textwrap |
19 | 24 | import time |
20 | 25 | import threading |
| 26 | + |
| 27 | + |
| 28 | +def _run_in_subprocess(body: str, conn_str: str) -> None: |
| 29 | + """Run a test body in a fresh Python process. |
| 30 | +
|
| 31 | + Some tests need to be the *first* to call ``pooling(...)`` in the |
| 32 | + process (the C++ ``enable_pooling`` is wrapped in ``std::call_once`` |
| 33 | + so only the first call's max_size/idle_timeout take effect). Running |
| 34 | + them in a subprocess gives each a clean process state. |
| 35 | +
|
| 36 | + The subprocess inherits the current ``DB_CONNECTION_STRING`` env var |
| 37 | + so the worker uses the same database. ``body`` must be a self-contained |
| 38 | + Python snippet that exits non-zero on failure (any uncaught assertion |
| 39 | + is fine). |
| 40 | + """ |
| 41 | + env = os.environ.copy() |
| 42 | + env["DB_CONNECTION_STRING"] = conn_str |
| 43 | + proc = subprocess.run( |
| 44 | + [sys.executable, "-c", textwrap.dedent(body)], |
| 45 | + env=env, |
| 46 | + capture_output=True, |
| 47 | + text=True, |
| 48 | + timeout=120, |
| 49 | + ) |
| 50 | + # Sentinel exit code 77 means the subprocess decided to skip |
| 51 | + # (e.g. the test prerequisite is unmet on this server, like missing |
| 52 | + # KILL permission). The reason is printed to stderr. |
| 53 | + if proc.returncode == 77: |
| 54 | + pytest.skip(proc.stderr.strip() or "Subprocess requested skip") |
| 55 | + if proc.returncode != 0: |
| 56 | + pytest.fail( |
| 57 | + "Subprocess test body failed\n" |
| 58 | + f"--- stdout ---\n{proc.stdout}\n" |
| 59 | + f"--- stderr ---\n{proc.stderr}" |
| 60 | + ) |
| 61 | + |
| 62 | + |
21 | 63 | import statistics |
22 | 64 | from mssql_python import connect, pooling |
23 | 65 | from mssql_python.pooling import PoolingManager |
@@ -314,82 +356,182 @@ def test_pool_release_overflow_disconnects_outside_mutex(conn_str): |
314 | 356 | conn3.close() |
315 | 357 |
|
316 | 358 |
|
317 | | -@pytest.mark.skip("Flaky test - idle timeout behavior needs investigation") |
318 | 359 | def test_pool_idle_timeout_removes_connections(conn_str): |
319 | | - """Test that idle_timeout removes connections from the pool after the timeout.""" |
320 | | - pooling(max_size=2, idle_timeout=1) |
321 | | - conn1 = connect(conn_str) |
322 | | - spid_list = [] |
323 | | - cursor1 = conn1.cursor() |
324 | | - cursor1.execute("SELECT @@SPID") |
325 | | - spid1 = cursor1.fetchone()[0] |
326 | | - spid_list.append(spid1) |
327 | | - conn1.close() |
328 | | - |
329 | | - # Wait for longer than idle_timeout |
330 | | - time.sleep(3) |
331 | | - |
332 | | - # Get a new connection, which should not reuse the previous SPID |
333 | | - conn2 = connect(conn_str) |
334 | | - cursor2 = conn2.cursor() |
335 | | - cursor2.execute("SELECT @@SPID") |
336 | | - spid2 = cursor2.fetchone()[0] |
337 | | - spid_list.append(spid2) |
338 | | - conn2.close() |
339 | | - |
340 | | - assert spid1 != spid2, "Idle timeout did not remove connection from pool" |
| 360 | + """Test that idle_timeout removes connections from the pool after the timeout. |
| 361 | +
|
| 362 | + Run in a subprocess so this test's pooling(idle_timeout=1) is the |
| 363 | + first call in the process — the C++ ``enable_pooling`` is wrapped in |
| 364 | + ``std::call_once``, so only the first call's settings take effect for |
| 365 | + the lifetime of the process. |
| 366 | +
|
| 367 | + A bare SPID-inequality assertion is unreliable: SQL Server is free to |
| 368 | + reassign a recently-freed SPID to the next session. So we identify a |
| 369 | + session by the (SPID, login_time) tuple from sys.dm_exec_sessions — |
| 370 | + login_time has millisecond resolution and is unique per physical |
| 371 | + connection. |
| 372 | + """ |
| 373 | + _run_in_subprocess( |
| 374 | + """ |
| 375 | + import os, time |
| 376 | + from mssql_python import connect, pooling |
| 377 | +
|
| 378 | + conn_str = os.environ["DB_CONNECTION_STRING"] |
| 379 | + pooling(max_size=2, idle_timeout=1) |
| 380 | +
|
| 381 | + def session_identity(conn): |
| 382 | + cur = conn.cursor() |
| 383 | + cur.execute( |
| 384 | + "SELECT @@SPID, " |
| 385 | + " (SELECT login_time FROM sys.dm_exec_sessions " |
| 386 | + " WHERE session_id = @@SPID)" |
| 387 | + ) |
| 388 | + spid, login_time = cur.fetchone() |
| 389 | + return (spid, login_time) |
| 390 | +
|
| 391 | + c1 = connect(conn_str) |
| 392 | + id1 = session_identity(c1) |
| 393 | + c1.close() |
| 394 | +
|
| 395 | + time.sleep(3) |
| 396 | +
|
| 397 | + c2 = connect(conn_str) |
| 398 | + id2 = session_identity(c2) |
| 399 | + c2.close() |
| 400 | +
|
| 401 | + assert id1 != id2, ( |
| 402 | + f"Idle timeout did not remove connection from pool: " |
| 403 | + f"got the same session both times {id1}" |
| 404 | + ) |
| 405 | + """, |
| 406 | + conn_str, |
| 407 | + ) |
341 | 408 |
|
342 | 409 |
|
343 | 410 | # ============================================================================= |
344 | 411 | # Error Handling and Recovery Tests |
345 | 412 | # ============================================================================= |
346 | 413 |
|
347 | 414 |
|
348 | | -@pytest.mark.skip( |
349 | | - "Test causes fatal crash - forcibly closing underlying connection leads to undefined behavior" |
350 | | -) |
351 | 415 | def test_pool_removes_invalid_connections(conn_str): |
352 | | - """Test that the pool removes connections that become invalid (simulate by closing underlying connection).""" |
353 | | - pooling(max_size=1, idle_timeout=30) |
354 | | - conn = connect(conn_str) |
355 | | - cursor = conn.cursor() |
356 | | - cursor.execute("SELECT 1") |
357 | | - # Simulate invalidation by forcibly closing the connection at the driver level |
358 | | - try: |
359 | | - # Try to access a private attribute or method to forcibly close the underlying connection |
360 | | - # This is implementation-specific; if not possible, skip |
361 | | - if hasattr(conn, "_conn") and hasattr(conn._conn, "close"): |
362 | | - conn._conn.close() |
363 | | - else: |
364 | | - pytest.skip("Cannot forcibly close underlying connection for this driver") |
365 | | - except Exception: |
366 | | - pass |
367 | | - # Safely close the connection, ignoring errors due to forced invalidation |
368 | | - try: |
369 | | - conn.close() |
370 | | - except RuntimeError as e: |
371 | | - if "not initialized" not in str(e): |
| 416 | + """Pool must replace a pooled connection whose server-side session has died. |
| 417 | +
|
| 418 | + Run in a subprocess so this test does not pollute the in-process pool |
| 419 | + state for sibling tests (KILL leaves dead pool entries that survive |
| 420 | + Python-side teardown because the C++ pool config is locked in for the |
| 421 | + lifetime of the process via ``std::call_once``). |
| 422 | +
|
| 423 | + Simulates the realistic failure mode (DBA KILL, failover, server-side |
| 424 | + idle timeout) by: |
| 425 | + 1. Opening two connections concurrently (distinct physical sessions) |
| 426 | + in autocommit mode. |
| 427 | + 2. Using one to KILL the other's server-side session out-of-band. |
| 428 | + 3. Returning both to the pool. |
| 429 | + 4. Re-acquiring repeatedly: every connection must work and the |
| 430 | + killed SPID must never reappear. |
| 431 | +
|
| 432 | + Only public APIs are used. |
| 433 | + """ |
| 434 | + _run_in_subprocess( |
| 435 | + """ |
| 436 | + import os |
| 437 | + import time |
| 438 | + from mssql_python import connect, pooling |
| 439 | +
|
| 440 | + conn_str = os.environ["DB_CONNECTION_STRING"] |
| 441 | + pooling(max_size=2, idle_timeout=30) |
| 442 | +
|
| 443 | + def session_identity(conn): |
| 444 | + cur = conn.cursor() |
| 445 | + cur.execute( |
| 446 | + "SELECT @@SPID, " |
| 447 | + " (SELECT login_time FROM sys.dm_exec_sessions " |
| 448 | + " WHERE session_id = @@SPID)" |
| 449 | + ) |
| 450 | + spid, login_time = cur.fetchone() |
| 451 | + return (spid, login_time) |
| 452 | +
|
| 453 | + # Step 1: two distinct, autocommit connections. Autocommit avoids |
| 454 | + # the implicit rollback in Connection.close(), which would |
| 455 | + # otherwise fail on the killed session and leak its pool slot. |
| 456 | + victim = connect(conn_str) |
| 457 | + admin = connect(conn_str) |
| 458 | + victim.autocommit = True |
| 459 | + admin.autocommit = True |
| 460 | +
|
| 461 | + victim_id = session_identity(victim) |
| 462 | + admin_id = session_identity(admin) |
| 463 | + assert victim_id != admin_id, ( |
| 464 | + "Pool handed out the same physical session to two concurrent " |
| 465 | + "acquires" |
| 466 | + ) |
| 467 | + victim_spid = victim_id[0] |
| 468 | +
|
| 469 | + # Step 2: admin KILLs the victim's session. Requires server |
| 470 | + # permission (ALTER ANY CONNECTION or sysadmin); on hosted/CI |
| 471 | + # databases the test login often lacks it, so skip gracefully. |
| 472 | + try: |
| 473 | + admin.cursor().execute(f"KILL {victim_spid}") |
| 474 | + except Exception as e: |
| 475 | + msg = str(e) |
| 476 | + if "permission" in msg.lower() or "KILL" in msg: |
| 477 | + import sys as _sys |
| 478 | + print( |
| 479 | + f"Skipping: KILL not permitted for this login: {msg}", |
| 480 | + file=_sys.stderr, |
| 481 | + ) |
| 482 | + victim.close() |
| 483 | + admin.close() |
| 484 | + _sys.exit(77) |
372 | 485 | raise |
373 | | - # Now, get a new connection from the pool and ensure it works |
374 | | - new_conn = connect(conn_str) |
375 | | - new_cursor = new_conn.cursor() |
376 | | - try: |
377 | | - new_cursor.execute("SELECT 1") |
378 | | - result = new_cursor.fetchone() |
379 | | - assert result is not None and result[0] == 1, "Pool did not remove invalid connection" |
380 | | - finally: |
381 | | - new_conn.close() |
| 486 | +
|
| 487 | + # KILL is processed asynchronously on the server, but we don't |
| 488 | + # need to wait for it here. The test's correctness contract is |
| 489 | + # "the killed (SPID, login_time) must never reappear in |
| 490 | + # subsequent acquires." Any session that gets handed back |
| 491 | + # later — whether the same SPID reused by the server or a |
| 492 | + # transparently-reconnected one — necessarily has a different |
| 493 | + # login_time, so the identity check below catches the only |
| 494 | + # failure mode that matters. |
| 495 | +
|
| 496 | + # Step 3: return both to the pool. |
| 497 | + victim.close() |
| 498 | + admin.close() |
| 499 | +
|
| 500 | + # Step 4: re-acquire from the pool. Each must be working; the |
| 501 | + # killed *physical session* (SPID, login_time) must never come |
| 502 | + # back. SQL Server is free to reassign the SPID number to a new |
| 503 | + # session, so SPID alone is not a reliable identity. |
| 504 | + seen_ids = set() |
| 505 | + for _ in range(4): |
| 506 | + c = connect(conn_str) |
| 507 | + try: |
| 508 | + seen_ids.add(session_identity(c)) |
| 509 | + assert c.cursor().execute("SELECT 1").fetchone()[0] == 1, ( |
| 510 | + "Pool handed out an unusable connection" |
| 511 | + ) |
| 512 | + finally: |
| 513 | + c.close() |
| 514 | + assert victim_id not in seen_ids, ( |
| 515 | + f"Pool returned the killed session {victim_id}; " |
| 516 | + f"saw sessions {seen_ids}" |
| 517 | + ) |
| 518 | + """, |
| 519 | + conn_str, |
| 520 | + ) |
382 | 521 |
|
383 | 522 |
|
384 | 523 | def test_pool_recovery_after_failed_connection(conn_str): |
385 | 524 | """Test that the pool recovers after a failed connection attempt.""" |
386 | 525 | pooling(max_size=1, idle_timeout=30) |
387 | | - # First, try to connect with a bad password (should fail) |
388 | | - if "Pwd=" in conn_str: |
389 | | - bad_conn_str = conn_str.replace("Pwd=", "Pwd=wrongpassword") |
390 | | - elif "Password=" in conn_str: |
391 | | - bad_conn_str = conn_str.replace("Password=", "Password=wrongpassword") |
392 | | - else: |
| 526 | + # First, try to connect with a bad password (should fail). |
| 527 | + # Match the password keyword case-insensitively since ODBC accepts any case. |
| 528 | + bad_conn_str = re.sub( |
| 529 | + r"(?i)(\b(?:pwd|password)\s*=)([^;]*)", |
| 530 | + r"\1wrongpassword", |
| 531 | + conn_str, |
| 532 | + count=1, |
| 533 | + ) |
| 534 | + if bad_conn_str == conn_str: |
393 | 535 | pytest.skip("No password found in connection string to modify") |
394 | 536 | with pytest.raises(Exception): |
395 | 537 | connect(bad_conn_str) |
|
0 commit comments