Skip to content

fix(godot): add initialization guard to prevent double plugin init #87

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

Merged
hyochan merged 4 commits into from
Apr 13, 2026
Merged

fix(godot): add initialization guard to prevent double plugin init #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0a18e96
fix(godot): add initialization guard to prevent double plugin init
hyochan Apr 13, 2026
844a7a6
test(godot): add tests for initialization guard and idempotent connec…
hyochan Apr 13, 2026
468d476
fix(godot): use static guard and remove cached connection shortcut
hyochan Apr 13, 2026
e283d4a
test(godot): improve guard tests with signal count verification
hyochan Apr 13, 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
49 changes: 48 additions & 1 deletion libraries/godot-iap/Example/tests/test_godot_iap.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,15 @@ func _ready() -> void:


func _run_all_tests() -> void:
# Connection tests
# Connection tests (run BEFORE guard tests to avoid state leakage)
test_init_connection_mock()
test_end_connection_mock()

# Initialization guard tests
test_ready_guard_prevents_double_init()
test_init_connection_idempotent()
test_no_duplicate_signal_connections()

# Product tests
await test_fetch_products_mock()

Expand All @@ -42,6 +47,48 @@ func _run_all_tests() -> void:
test_android_methods_mock()


# ============================================
# Initialization Guard Tests
# ============================================

func test_ready_guard_prevents_double_init() -> void:
# Static _is_initialized should be true after first _ready() call
_assert_true(GodotIapWrapper._is_initialized, "static _is_initialized should be true after _ready()")

# Count connected signals before second _ready() call
var connected_before = GodotIapPlugin.purchase_updated.get_connections().size()
GodotIapPlugin._ready()
var connected_after = GodotIapPlugin.purchase_updated.get_connections().size()

# Guard should prevent _init_native_plugin() from running again,
# so signal connection count must not increase
_assert_equal(connected_before, connected_after, "_ready() called twice should not add duplicate signal connections")
_assert_true(GodotIapWrapper._is_initialized, "static _is_initialized should still be true after second _ready()")


func test_init_connection_idempotent() -> void:
# Reset connection state to test fresh
GodotIapPlugin._is_connected = false

# Calling init_connection multiple times should not error
var result1 = GodotIapPlugin.init_connection()
_assert_true(result1, "First init_connection should return true")

var result2 = GodotIapPlugin.init_connection()
_assert_true(result2, "Second init_connection should return true")


func test_no_duplicate_signal_connections() -> void:
# After multiple _ready() calls, signals should not have duplicate connections
var purchase_updated_count = GodotIapPlugin.purchase_updated.get_connections().size()
var purchase_error_count = GodotIapPlugin.purchase_error.get_connections().size()

# In mock mode (no native plugin), there should be 0 native signal connections
# The key assertion: counts should be <= 1 (no duplicates)
_assert_true(purchase_updated_count <= 1, "purchase_updated should have at most 1 connection (got %d)" % purchase_updated_count)
_assert_true(purchase_error_count <= 1, "purchase_error should have at most 1 connection (got %d)" % purchase_error_count)


# ============================================
# Connection Tests (Mock Mode)
# ============================================
Expand Down
4 changes: 4 additions & 0 deletions libraries/godot-iap/addons/godot-iap/godot_iap.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,15 @@ signal developer_provided_billing_android(details: Dictionary)
# Native plugin reference
var _native_plugin: Object = null
var _is_connected: bool = false
static var _is_initialized: bool = false

# Platform detection
var _platform: String = ""

func _ready() -> void:
if _is_initialized:
return
_is_initialized = true
Comment on lines +36 to +38

@gemini-code-assist gemini-code-assist Bot Apr 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The use of an instance variable _is_initialized for this guard only prevents multiple calls to _ready() on the same instance (e.g., if a node is removed and re-added to the tree).

If the issue described in the PR (duplicate initialization from both Plugin and AutoLoad) results in two separate instances of the script being created, this guard will not prevent both from initializing the native plugin and connecting duplicate signals. While this is a good improvement for robustness, a more definitive fix for duplicate listeners would be to check _native_plugin.is_connected(signal_name, method) before calling connect() in the _connect_signals_* methods.

@hyochan hyochan Apr 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 468d476. Changed _is_initialized to static var so it guards across all instances.

Comment on lines +36 to +38

@coderabbitai coderabbitai Bot Apr 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Static guard currently leaves later GodotIapWrapper instances uninitialized.

Early return on _is_initialized skips per-instance state setup. A second wrapper instance can keep _native_plugin == null, so calls on that instance (e.g., init_connection) fail even though initialization already happened elsewhere.

Proposed fix 
 static var _is_initialized: bool = false
+static var _shared_native_plugin: Object = null
 
 # Platform detection
 var _platform: String = ""
 
 func _ready() -> void:
-	if _is_initialized:
-		return
-	_is_initialized = true
 	_platform = OS.get_name()
+
+	if _is_initialized:
+		# Reuse class-level initialized native handle for this instance.
+		_native_plugin = _shared_native_plugin
+		return
+
 	_init_native_plugin()
+	_shared_native_plugin = _native_plugin
+	_is_initialized = true
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@libraries/godot-iap/addons/godot-iap/godot_iap.gd` around lines 36 - 38, The
static guard _is_initialized causes later GodotIapWrapper instances to skip
per-instance setup leaving _native_plugin null; change the initialization logic
in the constructor/initializer so that while shared/global initialization
guarded by _is_initialized still runs only once, per-instance fields like
_native_plugin are always set (e.g., call the native plugin resolution and
assign _native_plugin and any other instance-specific state even when
_is_initialized is true) so methods such as init_connection on subsequent
GodotIapWrapper instances don't fail.

_platform = OS.get_name()
_init_native_plugin()

Expand Down
Loading