fix: remove constructor.Reset() from S7Client/S7Server destructors#107
Open
JordiB-Inserma wants to merge 2 commits into
Open
fix: remove constructor.Reset() from S7Client/S7Server destructors#107JordiB-Inserma wants to merge 2 commits into
JordiB-Inserma wants to merge 2 commits into
Conversation
S7Client::~S7Client() and S7Server::~S7Server() were calling constructor.Reset() on a static Nan::Persistent<FunctionTemplate>. This releases the persistent handle to the FunctionTemplate that V8 uses to create new instances of the object. Since constructor is a static class member shared across ALL instances, destroying ANY single instance invalidates the constructor template for every existing and future instance. When V8's GC collects the invalidated template and a new instance is created, S7Client::New tries to resolve the constructor via FunctionTemplate::GetFunction() which returns a stale/collected handle, causing a segfault. This crash is non-deterministic and depends on V8 GC timing. On Linux, the GC is more aggressive, making the crash frequent in applications with repeated create/destroy cycles (e.g. SCADA/HMI reconnection loops). The fix is simply removing constructor.Reset() from both destructors. The static FunctionTemplate should live for the lifetime of the module, not per-instance. References: - davenardella/snap7#29 (upstream pthread fix, prerequisite for full Linux stability) - davenardella/snap7#19 (server destructor race)
Add AGENTS.md, .opencode/, .claude/, .cursorrules, and .github/copilot-instructions.md to both .gitignore and .npmignore to keep AI assistant configuration files out of version control and npm package distribution.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
S7Client and S7Server destructors were calling
constructor.Reset()on a staticNan::Persistent<FunctionTemplate>, which destroys the V8 constructor template shared across all instances. This causes segfaults on Linux when applications create and destroyS7Clientinstances repeatedly (e.g. SCADA/HMI reconnection loops like FUXA).Root cause
constructoris a static class member initialized once during module load (Init). It stores theFunctionTemplatethat V8 uses to create new instances. Both~S7Client()and~S7Server()were callingconstructor.Reset(), which releases the persistent handle to the static FunctionTemplate.Since this is a static resource shared across ALL instances:
new snap7.S7Client()— works, FunctionTemplate aliveconstructor.Reset()destroys the static templatenew snap7.S7Client()→ V8's internal constructor resolution fails → SEGFAULTThe crash is non-deterministic and depends on V8 GC timing. On Linux, the GC is more aggressive, making this crash frequent in production SCADA systems.
Fix
Remove
constructor.Reset()from both~S7Client()(node_snap7_client.cpp:722) and~S7Server()(node_snap7_server.cpp:682). The static FunctionTemplate should live for the lifetime of the module, not per-instance.Changes
src/node_snap7_client.cppconstructor.Reset()from~S7Client()src/node_snap7_server.cppconstructor.Reset()from~S7Server()Stack trace before fix (Linux)
Related