mirror of
https://github.com/electron/electron.git
synced 2026-04-10 03:01:51 -04:00
fix: crash calling OSR shared texture release() after texture GC'd (#50473)
The weak persistent tracking the OffscreenReleaseHolderMonitor was tied to the texture object, but the release() closure holds a raw pointer to the monitor via its v8::External data. If JS retained texture.release while dropping the texture itself, the monitor would be freed on GC and a later release() call would crash. Track the release function instead of the texture object. Since the texture holds release as a property, this keeps the monitor alive as long as either is reachable.
This commit is contained in:
@@ -155,9 +155,12 @@ v8::Local<v8::Value> Converter<electron::OffscreenSharedTextureValue>::ToV8(
|
||||
root.Set("textureInfo", ConvertToV8(isolate, dict));
|
||||
auto root_local = ConvertToV8(isolate, root);
|
||||
|
||||
// Create a persistent reference of the object, so that we can check the
|
||||
// monitor again when GC collects this object.
|
||||
auto* tex_persistent = monitor->CreatePersistent(isolate, root_local);
|
||||
// Create a weak persistent that tracks the release function rather than the
|
||||
// texture object. The release function holds a raw pointer to |monitor| via
|
||||
// its v8::External data, so |monitor| must outlive it. Since the texture
|
||||
// keeps |release| alive via its property, this also covers the case where
|
||||
// the texture itself is leaked without calling release().
|
||||
auto* tex_persistent = monitor->CreatePersistent(isolate, releaser);
|
||||
tex_persistent->SetWeak(
|
||||
monitor,
|
||||
[](const v8::WeakCallbackInfo<OffscreenReleaseHolderMonitor>& data) {
|
||||
|
||||
@@ -6963,6 +6963,54 @@ describe('BrowserWindow module', () => {
|
||||
expect(w.webContents.frameRate).to.equal(30);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shared texture', () => {
|
||||
const v8Util = process._linkedBinding('electron_common_v8_util');
|
||||
|
||||
it('does not crash when release() is called after the texture is garbage collected', async () => {
|
||||
const sw = new BrowserWindow({
|
||||
width: 100,
|
||||
height: 100,
|
||||
show: false,
|
||||
webPreferences: {
|
||||
backgroundThrottling: false,
|
||||
offscreen: {
|
||||
useSharedTexture: true
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
const paint = once(sw.webContents, 'paint') as Promise<[any, Electron.Rectangle, Electron.NativeImage]>;
|
||||
sw.loadFile(path.join(fixtures, 'api', 'offscreen-rendering.html'));
|
||||
const [event] = await paint;
|
||||
sw.webContents.stopPainting();
|
||||
|
||||
if (!event.texture) {
|
||||
// GPU shared texture not available on this host; skip.
|
||||
sw.destroy();
|
||||
return;
|
||||
}
|
||||
|
||||
// Keep only the release closure and drop the owning texture object.
|
||||
const staleRelease = event.texture.release;
|
||||
const weakTexture = new WeakRef(event.texture);
|
||||
event.texture = undefined;
|
||||
|
||||
// Force GC until the texture object is collected.
|
||||
let collected = false;
|
||||
for (let i = 0; i < 30 && !collected; ++i) {
|
||||
await setTimeout();
|
||||
v8Util.requestGarbageCollectionForTesting();
|
||||
collected = weakTexture.deref() === undefined;
|
||||
}
|
||||
expect(collected).to.be.true('texture should be garbage collected');
|
||||
|
||||
// This should return safely and not crash the main process.
|
||||
expect(() => staleRelease()).to.not.throw();
|
||||
|
||||
sw.destroy();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('offscreen rendering with device scale factor', () => {
|
||||
|
||||
Reference in New Issue
Block a user