Skip to content

Commit 1d507fd

Browse files
caohy1988claude
andcommitted
fix: skip false-positive fork detection after pickle in BQ analytics plugin
When the plugin is deployed via Vertex AI Agent Engine, it is pickled for transport and unpickled on the server. __getstate__ sets _init_pid = 0 as a pickle sentinel. On the server, _ensure_started() checks os.getpid() != self._init_pid, which always evaluates to True since os.getpid() is never 0. This triggers _reset_runtime_state() on every cold start even though no fork happened, producing a misleading "Fork detected (parent PID 0, child PID xx)" warning and adding unnecessary startup latency from tearing down and re-creating gRPC state that was already clear. The fix distinguishes "unpickled, never initialized" (_init_pid == 0) from "forked from a different process" (_init_pid != 0 and _init_pid != os.getpid()). Real forks are still detected by both os.register_at_fork (line 108) and this PID check. Related: haiyuan-eng-google/BigQuery-Agent-Analytics-SDK#86 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3e282d2 commit 1d507fd

2 files changed

Lines changed: 80 additions & 1 deletion

File tree

src/google/adk/plugins/bigquery_agent_analytics_plugin.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2653,7 +2653,14 @@ async def __aexit__(self, exc_type, exc_val, exc_tb) -> None:
26532653

26542654
async def _ensure_started(self, **kwargs) -> None:
26552655
"""Ensures that the plugin is started and initialized."""
2656-
if os.getpid() != self._init_pid:
2656+
# _init_pid == 0 means the plugin was unpickled and has never been
2657+
# initialized in this process (the pickle sentinel set by
2658+
# __getstate__). Skip the fork reset in that case — no fork
2659+
# happened, and _started is already False so _lazy_setup will run.
2660+
# Real forks are caught by os.register_at_fork (line 108) and by
2661+
# this check when _init_pid is a real (non-zero) PID from a
2662+
# different process.
2663+
if self._init_pid != 0 and os.getpid() != self._init_pid:
26572664
self._reset_runtime_state()
26582665
if not self._started:
26592666
# Kept original lock name as it was not explicitly changed.

tests/unittests/plugins/test_bigquery_agent_analytics_plugin.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7408,3 +7408,75 @@ async def test_view_error_still_logged(
74087408
) as plugin:
74097409
await plugin._ensure_started()
74107410
assert plugin._started
7411+
7412+
7413+
# ================================================================
7414+
# TEST CLASS: Fork detection after pickle (Issue #86)
7415+
# ================================================================
7416+
class TestForkDetectionAfterPickle:
7417+
"""Tests that unpickled plugins do not false-positive fork detection."""
7418+
7419+
@pytest.mark.asyncio
7420+
async def test_no_reset_after_unpickle(
7421+
self,
7422+
mock_auth_default,
7423+
mock_bq_client,
7424+
mock_write_client,
7425+
mock_to_arrow_schema,
7426+
mock_asyncio_to_thread,
7427+
):
7428+
"""Unpickled plugin does not trigger _reset_runtime_state."""
7429+
import pickle
7430+
7431+
config = bigquery_agent_analytics_plugin.BigQueryLoggerConfig(
7432+
create_views=False,
7433+
)
7434+
plugin = bigquery_agent_analytics_plugin.BigQueryAgentAnalyticsPlugin(
7435+
PROJECT_ID, DATASET_ID, table_id=TABLE_ID, config=config
7436+
)
7437+
# Pickle round-trip simulates Vertex AI Agent Engine deployment
7438+
pickled = pickle.dumps(plugin)
7439+
unpickled = pickle.loads(pickled)
7440+
7441+
assert unpickled._init_pid == 0 # pickle sentinel
7442+
7443+
with mock.patch.object(unpickled, "_reset_runtime_state") as mock_reset:
7444+
await unpickled._ensure_started()
7445+
# Should NOT have called _reset_runtime_state because
7446+
# _init_pid == 0 means "unpickled, never initialized"
7447+
mock_reset.assert_not_called()
7448+
7449+
assert unpickled._started
7450+
await unpickled.shutdown()
7451+
7452+
@pytest.mark.asyncio
7453+
async def test_reset_on_real_fork(
7454+
self,
7455+
mock_auth_default,
7456+
mock_bq_client,
7457+
mock_write_client,
7458+
mock_to_arrow_schema,
7459+
mock_asyncio_to_thread,
7460+
):
7461+
"""Plugin detects real fork when _init_pid is a real non-zero PID."""
7462+
config = bigquery_agent_analytics_plugin.BigQueryLoggerConfig(
7463+
create_views=False,
7464+
)
7465+
async with managed_plugin(
7466+
project_id=PROJECT_ID,
7467+
dataset_id=DATASET_ID,
7468+
table_id=TABLE_ID,
7469+
config=config,
7470+
) as plugin:
7471+
await plugin._ensure_started()
7472+
# Simulate a fork: set _init_pid to a different real PID
7473+
plugin._init_pid = 99999
7474+
plugin._started = True # pretend it was started in parent
7475+
7476+
with mock.patch.object(
7477+
plugin, "_reset_runtime_state", wraps=plugin._reset_runtime_state
7478+
) as mock_reset:
7479+
await plugin._ensure_started()
7480+
# Should have called _reset_runtime_state because
7481+
# _init_pid is a real PID different from os.getpid()
7482+
mock_reset.assert_called_once()

0 commit comments

Comments
 (0)