Repository navigation
fix(geometry): use Nan::CopyBuffer for V8 Memory Sandbox and Electron 32+ compatibility - #382
Open
CoryAlbrecht wants to merge 3 commits into
Open
CoryAlbrecht wants to merge 3 commits into
CoryAlbrecht wants to merge 3 commits into
Conversation
Owner
|
Are you using |
This branch has not been deployed
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
Fixes part of #28 (V8 memory sandbox and backing store pointer compression crash under Electron 32+).
In Electron 32+ (and any Node.js runtime built with
v8_enable_sandbox = true), Chromium enforces the V8 Memory Sandbox, restricting allArrayBufferbacking stores to a 4-GB pointer compression cage. Wrapping nativemallocor GDAL heap buffers withNan::NewBuffer()triggers an uncatchable fatal abort:This PR replaces unmanaged
Nan::NewBufferallocations withNan::CopyBuffer()and immediate native deallocation:Files changed:
src/geometry/gdal_geometry.cpp: ReplaceNan::NewBufferwithNan::CopyBufferinexportToWKBand free nativemallocbuffer immediately.src/utils/typed_array.cpp: ReplaceNan::NewBufferwithNan::CopyBufferinTypedArray::Newso typed array backing stores reside within the V8 pointer cage.src/gdal_memfile.cpp: ReplaceNan::NewBufferwithNan::CopyBufferinMemfile::vsimemRelease, immediately freeing the unlinked GDAL buffer withCPLFree(data).test/api_geometry.test.ts: Add regression unit tests fortoWKB()andtoWKBAsync()across Point (MSB/LSB), LineString, and Polygon.Evidence
Under Electron 32+, invoking
geom.toWKB()crashes the process immediately:All geometries export valid
Bufferinstances under Electron 32+ and standard Node.js without memory errors:test/api_geometry.test.tsverifying binary WKB headers, Point MSB/LSB orders, LineString coordinates, and Polygon ring geometry round-trips. Memory copy overhead is negligible (~0.05 ms per feature batch).Merge Danger
Door: Two-way
Fully backwards compatible. Public TypeScript interfaces, method signatures, and return types (
Buffer,Promise<Buffer>) are identical.Blast Radius: Minimal
Only changes how native C/GDAL byte buffers are transferred into V8 memory for
exportToWKB,TypedArray::New, andvsimemRelease.