Skip to content

Commit 8c2340d

Browse files
bewithgauravbewithgaurav
committed
Merge branch 'main' of https://github.com/microsoft/mssql-python into bewithgaurav/fix-conninfo-utf-decoding
2 parents 93091f2 + cc3940c commit 8c2340d

File tree

8 files changed

+1614
-40
lines changed

8 files changed

+1614
-40
lines changed

‎OneBranchPipelines/github-ado-sync.yml‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
# GitHub-to-ADO Sync Pipeline
2+
# Syncs main branch from public GitHub to internal Azure DevOps daily at 5pm IST
3+
4+
name: GitHub-Sync-$(Date:yyyyMMdd)$(Rev:.r)
5+
6+
schedules:
7+
- cron: "30 11 * * *"
8+
displayName: "Daily sync at 5pm IST"
9+
branches:
10+
include:
11+
- main
12+
always: true
13+
14+
trigger: none
15+
pr: none
16+
17+
jobs:
18+
- job: SyncFromGitHub
19+
displayName: 'Sync main branch from GitHub'
20+
pool:
21+
vmImage: 'windows-latest'
22+
23+
steps:
24+
- checkout: none
25+
26+
- task: CmdLine@2
27+
displayName: 'Clone GitHub repo'
28+
inputs:
29+
script: git clone https://github.com/microsoft/mssql-python.git repo-dir -b main
30+
workingDirectory: $(Agent.TempDirectory)
31+
32+
- task: CmdLine@2
33+
displayName: 'Add Azure DevOps remote'
34+
inputs:
35+
script: git remote add azdo-mirror https://$(System.AccessToken)@sqlclientdrivers.visualstudio.com/mssql-python/_git/mssql-python
36+
workingDirectory: $(Agent.TempDirectory)/repo-dir
37+
38+
- task: CmdLine@2
39+
displayName: 'Fetch ADO repo'
40+
inputs:
41+
script: git fetch azdo-mirror
42+
workingDirectory: $(Agent.TempDirectory)/repo-dir
43+
44+
- task: CmdLine@2
45+
displayName: 'Create timestamped sync branch'
46+
inputs:
47+
script: |
48+
echo Getting current timestamp...
49+
powershell -Command "Get-Date -Format 'yyyyMMdd-HHmmss'" > timestamp.txt
50+
set /p TIMESTAMP=<timestamp.txt
51+
set SYNC_BRANCH=github-sync-%TIMESTAMP%
52+
echo %SYNC_BRANCH% > branchname.txt
53+
echo Creating sync branch: %SYNC_BRANCH%
54+
git fetch azdo-mirror
55+
git show-ref --verify --quiet refs/remotes/azdo-mirror/main
56+
if %ERRORLEVEL% EQU 0 (
57+
git checkout -b %SYNC_BRANCH% -t azdo-mirror/main
58+
) else (
59+
echo azdo-mirror/main does not exist. Exiting.
60+
exit /b 1
61+
)
62+
echo ##vso[task.setvariable variable=SYNC_BRANCH;isOutput=true]%SYNC_BRANCH%
63+
workingDirectory: $(Agent.TempDirectory)/repo-dir
64+
65+
- task: CmdLine@2
66+
displayName: 'Reset branch to match GitHub main exactly'
67+
inputs:
68+
script: |
69+
git -c user.email="sync@microsoft.com" -c user.name="ADO Sync Bot" reset --hard origin/main
70+
workingDirectory: $(Agent.TempDirectory)/repo-dir
71+
72+
- task: CmdLine@2
73+
displayName: 'Push branch to Azure DevOps'
74+
inputs:
75+
script: |
76+
set /p SYNC_BRANCH=<branchname.txt
77+
echo Pushing branch: %SYNC_BRANCH%
78+
79+
git config user.email "sync@microsoft.com"
80+
git config user.name "ADO Sync Bot"
81+
82+
git push azdo-mirror %SYNC_BRANCH% --set-upstream
83+
84+
if %ERRORLEVEL% EQU 0 (
85+
echo Branch pushed successfully!
86+
) else (
87+
echo ERROR: Push failed!
88+
exit /b 1
89+
)
90+
workingDirectory: $(Agent.TempDirectory)/repo-dir
91+
92+
- task: CmdLine@2
93+
displayName: 'Create pull request'
94+
inputs:
95+
script: |
96+
echo Installing Azure DevOps extension...
97+
call az extension add --name azure-devops --only-show-errors
98+
99+
echo Setting up authentication...
100+
set AZURE_DEVOPS_EXT_PAT=$(System.AccessToken)
101+
102+
echo Configuring Azure DevOps defaults...
103+
call az devops configure --defaults organization=https://sqlclientdrivers.visualstudio.com project=mssql-python
104+
105+
set /p SYNC_BRANCH=<branchname.txt
106+
echo Creating PR from branch: %SYNC_BRANCH% to main
107+
108+
call az repos pr create --source-branch %SYNC_BRANCH% --target-branch main --title "ADO-GitHub Sync - %date%" --description "Automated sync from GitHub main branch" --auto-complete true --squash true --delete-source-branch true --output table
109+
110+
if %ERRORLEVEL% EQU 0 (
111+
echo PR created successfully!
112+
echo Adding reviewers...
113+
call az repos pr list --source-branch %SYNC_BRANCH% --target-branch main --status active --output tsv --query "[0].pullRequestId" > pr_id.txt
114+
set /p PR_ID=<pr_id.txt
115+
call az repos pr reviewer add --id %PR_ID% --reviewers gargsaumya@microsoft.com sharmag@microsoft.com sumit.sarabhai@microsoft.com jathakkar@microsoft.com spaitandi@microsoft.com --output table
116+
if %ERRORLEVEL% EQU 0 (
117+
echo Reviewers added successfully!
118+
)
119+
del pr_id.txt
120+
) else (
121+
echo ERROR: Failed to create PR
122+
exit /b 1
123+
)
124+
125+
if exist timestamp.txt del timestamp.txt
126+
if exist branchname.txt del branchname.txt
127+
workingDirectory: $(Agent.TempDirectory)/repo-dir

‎mssql_python/__init__.py‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@
44
This module initializes the mssql_python package.
55
"""
66

7+
import atexit
78
import sys
9+
import threading
810
import types
11+
import weakref
912
from typing import Dict
1013

1114
# Import settings from helpers to avoid circular imports
@@ -67,6 +70,49 @@
6770
# Pooling
6871
from .pooling import PoolingManager
6972

73+
# Global registry for tracking active connections (using weak references)
74+
_active_connections = weakref.WeakSet()
75+
_connections_lock = threading.Lock()
76+
77+
78+
def _register_connection(conn):
79+
"""Register a connection for cleanup before shutdown."""
80+
with _connections_lock:
81+
_active_connections.add(conn)
82+
83+
84+
def _cleanup_connections():
85+
"""
86+
Cleanup function called by atexit to close all active connections.
87+
88+
This prevents resource leaks during interpreter shutdown by ensuring
89+
all ODBC handles are freed in the correct order before Python finalizes.
90+
"""
91+
# Make a copy of the connections to avoid modification during iteration
92+
with _connections_lock:
93+
connections_to_close = list(_active_connections)
94+
95+
for conn in connections_to_close:
96+
try:
97+
# Check if connection is still valid and not closed
98+
if hasattr(conn, "_closed") and not conn._closed:
99+
# Close will handle both cursors and the connection
100+
conn.close()
101+
except Exception as e:
102+
# Log errors during shutdown cleanup for debugging
103+
# We're prioritizing crash prevention over error propagation
104+
try:
105+
driver_logger.error(
106+
f"Error during connection cleanup at shutdown: {type(e).__name__}: {e}"
107+
)
108+
except Exception:
109+
# If logging fails during shutdown, silently ignore
110+
pass
111+
112+
113+
# Register cleanup function to run before Python exits
114+
atexit.register(_cleanup_connections)
115+
70116
# GLOBALS
71117
# Read-Only
72118
apilevel: str = "2.0"

‎mssql_python/connection.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from typing import Any, Dict, Optional, Union, List, Tuple, Callable, TYPE_CHECKING
1818
import threading
1919

20+
import mssql_python
2021
from mssql_python.cursor import Cursor
2122
from mssql_python.helpers import (
2223
add_driver_to_connection_str,
@@ -312,6 +313,22 @@ def __init__(
312313
)
313314
self.setautocommit(autocommit)
314315

316+
# Register this connection for cleanup before Python shutdown
317+
# This ensures ODBC handles are freed in correct order, preventing leaks
318+
try:
319+
if hasattr(mssql_python, "_register_connection"):
320+
mssql_python._register_connection(self)
321+
except AttributeError as e:
322+
# If registration fails, continue - cleanup will still happen via __del__
323+
logger.warning(
324+
f"Failed to register connection for shutdown cleanup: {type(e).__name__}: {e}"
325+
)
326+
except Exception as e:
327+
# Catch any other unexpected errors during registration
328+
logger.error(
329+
f"Unexpected error during connection registration: {type(e).__name__}: {e}"
330+
)
331+
315332
def _construct_connection_string(self, connection_str: str = "", **kwargs: Any) -> str:
316333
"""
317334
Construct the connection string by parsing, validating, and merging parameters.

‎mssql_python/pybind/ddbc_bindings.cpp‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,11 +1163,15 @@ void SqlHandle::free() {
11631163
// Check if Python is shutting down using centralized helper function
11641164
bool pythonShuttingDown = is_python_finalizing();
11651165

1166-
// CRITICAL FIX: During Python shutdown, don't free STMT handles as
1167-
// their parent DBC may already be freed This prevents segfault when
1168-
// handles are freed in wrong order during interpreter shutdown Type 3 =
1169-
// SQL_HANDLE_STMT, Type 2 = SQL_HANDLE_DBC, Type 1 = SQL_HANDLE_ENV
1170-
if (pythonShuttingDown && _type == 3) {
1166+
// RESOURCE LEAK MITIGATION:
1167+
// When handles are skipped during shutdown, they are not freed, which could
1168+
// cause resource leaks. However, this is mitigated by:
1169+
// 1. Python-side atexit cleanup (in __init__.py) that explicitly closes all
1170+
// connections before shutdown, ensuring handles are freed in correct order
1171+
// 2. OS-level cleanup at process termination recovers any remaining resources
1172+
// 3. This tradeoff prioritizes crash prevention over resource cleanup, which
1173+
// is appropriate since we're already in shutdown sequence
1174+
if (pythonShuttingDown && (_type == SQL_HANDLE_STMT || _type == SQL_HANDLE_DBC)) {
11711175
_handle = nullptr; // Mark as freed to prevent double-free attempts
11721176
return;
11731177
}

‎tests/test_002_types.py‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import pytest
22
import datetime
33
import time
4+
import os
45
from mssql_python.type import (
56
STRING,
67
BINARY,
@@ -539,7 +540,10 @@ def test_invalid_surrogate_handling():
539540
# In UTF-16, high surrogates (0xD800-0xDBFF) must be followed by low surrogates
540541
try:
541542
# Create a connection string that would exercise the conversion path
542-
conn_str = "Server=test_server;Database=TestDB;UID=user;PWD=password"
543+
# Use environment variables or placeholder values to avoid SEC101/037 security warnings
544+
test_server = os.getenv("TEST_SERVER", "testserver")
545+
test_db = os.getenv("TEST_DATABASE", "TestDB")
546+
conn_str = f"Server={test_server};Database={test_db};Trusted_Connection=yes"
543547
conn = mssql_python.connect(conn_str, autoconnect=False)
544548
conn.close()
545549
except Exception:
@@ -548,7 +552,10 @@ def test_invalid_surrogate_handling():
548552
# Low surrogate without high surrogate (invalid)
549553
# In UTF-16, low surrogates (0xDC00-0xDFFF) must be preceded by high surrogates
550554
try:
551-
conn_str = "Server=test;Database=DB;ApplicationName=TestApp;UID=u;PWD=p"
555+
test_server = os.getenv("TEST_SERVER", "testserver")
556+
conn_str = (
557+
f"Server={test_server};Database=DB;ApplicationName=TestApp;Trusted_Connection=yes"
558+
)
552559
conn = mssql_python.connect(conn_str, autoconnect=False)
553560
conn.close()
554561
except Exception:
@@ -564,7 +571,7 @@ def test_invalid_surrogate_handling():
564571

565572
for test_str in emoji_tests:
566573
try:
567-
conn_str = f"Server=test;{test_str};UID=user;PWD=pass"
574+
conn_str = f"Server=test;{test_str};Trusted_Connection=yes"
568575
conn = mssql_python.connect(conn_str, autoconnect=False)
569576
conn.close()
570577
except Exception:

0 commit comments

Comments
 (0)