Skip to content

fix: keep async query callback closures alive to prevent use-after-free crash #390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
qevolg wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: keep async query callback closures alive to prevent use-after-free crash #390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dfe4ab8
fix: keep async query callback closures alive to prevent use-after-fr…
qevolg Jun 29, 2026
4f5c225
ci: build taosadapter from source so Build tests run and coverage upl…
qevolg Jul 1, 2026
01aff0e
ci: deselect live TDengine Cloud tests in Build
qevolg Jul 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 52 additions & 11 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ jobs:
- name: Build tools
run: |
sudo apt-get update -y
sudo apt-get install -y build-essential libgeos-dev
sudo apt-get install -y build-essential cmake \
libgeos-dev libjansson-dev libsnappy-dev liblzma-dev libz-dev \
zlib1g pkg-config libssl-dev gawk

- name: Setup Python
run: sudo apt-get install -y python3 python3-pip python-is-python3
Expand Down Expand Up @@ -98,34 +100,72 @@ jobs:
rm build -rf
mkdir build
cd build
cmake ../ -DBUILD_JDBC=false -DCMAKE_INSTALL_PREFIX:PATH=`realpath ../../local/` -DBUILD_HTTP=false -DBUILD_DEPENDENCY_TESTS=false
make -j14
cmake ../ -DBUILD_JDBC=false -DBUILD_TOOLS=false -DBUILD_TEST=off -DBUILD_DEPENDENCY_TESTS=false -DBUILD_CONTRIB=ON
make -j 4
sudo make install
sudo ldconfig
which taosd
ls -alh /etc/taos/
cd ../../

# taosadapter is no longer produced by the TDengine build, so build it from source.
- name: Checkout taosadapter
uses: actions/checkout@v4
with:
repository: "taosdata/taosadapter"
path: "taosadapter"
ref: ${{ steps.determine-branch.outputs.value }}

- name: Set up Go
uses: actions/setup-go@v6
with:
go-version: "1.26.0"
cache-dependency-path: taosadapter/go.sum

- name: Build and install taosadapter
run: |
cd taosadapter
go mod download
go build -o taosadapter
sudo install -m 755 taosadapter /usr/local/bin/taosadapter
taosadapter --version

- name: Start TDengine 3.0
run: |
tree TDengine_v3/build/build/
export C_INCLUDE_PATH=$PWD/TDengine_v3/build/build/bin
export LD_LIBRARY_PATH=$PWD/TDengine_v3/build/build/lib
mkdir -p /tmp/taos/v3/log /tmp/taos/v3/data
printf "dataDir /tmp/taos/v3/data\nlogDir /tmp/taos/v3/log\ndebugFlag 135\n" |sudo tee /etc/taos/taos.cfg
TAOS_SUPPORT_VNODES=256 ./TDengine_v3/build/build/bin/taosd &
./TDengine_v3/build/build/bin/taosadapter &
printf "dataDir /tmp/taos/v3/data\nlogDir /tmp/taos/v3/log\nsupportVnodes 256\n" | sudo tee /etc/taos/taos.cfg
nohup sudo taosd > /tmp/taos/v3/taosd.out 2>&1 &
nohup sudo taosadapter > /tmp/taos/v3/taosadapter.out 2>&1 &

#----------------------------------------------
# run test suite
#----------------------------------------------
- name: Wait for TDengine
run: |
for i in $(seq 1 60); do
if curl -s -m 2 http://localhost:6041/-/ping > /dev/null 2>&1; then
echo "taosadapter is up"
break
fi
echo "waiting for taosadapter ($i/60) ..."
sleep 2
done

- name: Run Tests
env:
LD_LIBRARY_PATH: $PWD/TDengine_v3/build/build/lib
TDENGINE_URL: localhost:6041
TDENGINE_TEST_USERNAME: ${{ secrets.TDENGINE_TEST_USERNAME }}
TDENGINE_TEST_PASSWORD: ${{ secrets.TDENGINE_TEST_PASSWORD }}
run: |
# Fall back to default credentials when the secrets are unavailable (e.g. fork PRs).
export TDENGINE_TEST_USERNAME="${TDENGINE_TEST_USERNAME:-root}"
export TDENGINE_TEST_PASSWORD="${TDENGINE_TEST_PASSWORD:-taosdata}"
curl -L -u "$TDENGINE_TEST_USERNAME:$TDENGINE_TEST_PASSWORD" -d "show databases" localhost:6041/rest/sql
poetry run pytest --cov-report term --cov-report html --cov-report xml --cov=taos --cov=taosrest --cov-append tests
# Deselect tests that hit live TDengine Cloud endpoints; they depend on
# external services and make the local build non-deterministic.
poetry run pytest --cov-report term --cov-report html --cov-report xml --cov=taos --cov=taosrest --cov-append tests \
--deselect tests/test_rest_connection.py::test_token \
--deselect tests/test_rest_connection.py::test_wrong_token

- name: Upload taosd logs
if: failure()
Expand All @@ -139,6 +179,7 @@ jobs:
uses: codecov/codecov-action@v4
with:
files: ./coverage.xml
fail_ci_if_error: false
token: ${{ secrets.CODECOV_TOKEN }}

- name: Build Artifacts
Expand Down
37 changes: 34 additions & 3 deletions taos/cinterface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,13 +511,40 @@ def taos_query_with_reqid(connection, sql, req_id):


async_query_callback_type = CFUNCTYPE(None, c_void_p, c_void_p, c_int)


# The async APIs (taos_query_a / taos_query_a_with_reqid / taos_fetch_rows_a) invoke
# the callback later, from a native client worker thread. The ctypes closure that
# wraps the Python callback must stay alive until then; otherwise it can be garbage
# collected while the native side still holds the function pointer, which leads to a
# segfault (use-after-free) when the callback is finally fired. Keep a reference to
# the closure of each in-flight callback and release it right after it is invoked.
_inflight_async_callbacks = {}


def _retain_async_callback(callback_type, callback):
# type: (type, callable) -> object
def wrapper(*args):
try:
callback(*args)
finally:
_inflight_async_callbacks.pop(token, None)

closure = callback_type(wrapper)
token = id(closure)
_inflight_async_callbacks[token] = closure
return closure
Comment on lines +525 to +536

@gemini-code-assist gemini-code-assist Bot Jun 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If callback is passed as None, the original behavior of the ctypes wrapper was to pass a NULL pointer to the C library (since callback_type(None) evaluates to a NULL pointer).

With the current implementation, passing None will wrap it in wrapper, creating a valid function pointer. When the native library invokes this callback, it will attempt to call None(*args), resulting in a TypeError: 'NoneType' object is not callable inside the callback thread.

To preserve the original behavior and handle None callbacks defensively, we should check if callback is None and return None immediately.

Suggested change
def _retain_async_callback(callback_type, callback):
# type: (type, callable) -> object
def wrapper(*args):
try:
callback(*args)
finally:
_inflight_async_callbacks.pop(token, None)
closure = callback_type(wrapper)
token = id(closure)
_inflight_async_callbacks[token] = closure
return closure
def _retain_async_callback(callback_type, callback):
# type: (type, callable) -> object
if callback is None:
return None
def wrapper(*args):
try:
callback(*args)
finally:
_inflight_async_callbacks.pop(token, None)
closure = callback_type(wrapper)
token = id(closure)
_inflight_async_callbacks[token] = closure
return closure



_libtaos.taos_query_a.restype = None
_libtaos.taos_query_a.argtypes = c_void_p, c_char_p, async_query_callback_type, c_void_p


def taos_query_a(connection, sql, callback, param):
# type: (c_void_p, str, async_query_callback_type, c_void_p) -> None
_libtaos.taos_query_a(connection, c_char_p(sql.encode("utf-8")), async_query_callback_type(callback), param)
_libtaos.taos_query_a(
connection, c_char_p(sql.encode("utf-8")), _retain_async_callback(async_query_callback_type, callback), param
)


# add req_id for async query
Expand Down Expand Up @@ -548,7 +575,11 @@ def taos_query_a_with_reqid(connection, sql, callback, param, req_id):
"""
_check_if_supported()
_libtaos.taos_query_a_with_reqid(
connection, c_char_p(sql.encode("utf-8")), async_query_with_reqid_callback_type(callback), param, req_id
connection,
c_char_p(sql.encode("utf-8")),
_retain_async_callback(async_query_with_reqid_callback_type, callback),
param,
req_id,
)


Expand All @@ -559,7 +590,7 @@ def taos_query_a_with_reqid(connection, sql, callback, param, req_id):

def taos_fetch_rows_a(result, callback, param):
# type: (c_void_p, async_fetch_rows_callback_type, c_void_p) -> None
_libtaos.taos_fetch_rows_a(result, async_fetch_rows_callback_type(callback), param)
_libtaos.taos_fetch_rows_a(result, _retain_async_callback(async_fetch_rows_callback_type, callback), param)


def taos_affected_rows(result):
Expand Down
Loading