Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(91)

Issue 5350367445385216: Fix the crashing (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 1 month ago by Oleksandr
Modified:
5 years, 1 month ago
Visibility:
Public.

Description

Fix the crashing

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -5 lines) Patch
M include/AdblockPlus/V8ValueHolder.h View 1 chunk +1 line, -5 lines 1 comment Download

Messages

Total messages: 4
Oleksandr
Fixes the occasional crashes in libadblockplus
5 years, 1 month ago (2014-07-01 22:58:49 UTC) #1
Oleksandr
5 years, 1 month ago (2014-07-02 09:20:10 UTC) #2
Wladimir Palant
http://codereview.adblockplus.org/5350367445385216/diff/5629499534213120/include/AdblockPlus/V8ValueHolder.h File include/AdblockPlus/V8ValueHolder.h (right): http://codereview.adblockplus.org/5350367445385216/diff/5629499534213120/include/AdblockPlus/V8ValueHolder.h#newcode54 include/AdblockPlus/V8ValueHolder.h:54: reset(isolate, v8::Persistent<T>()); I don't get it, with this change ...
5 years, 1 month ago (2014-07-02 09:35:12 UTC) #3
Oleksandr
5 years, 1 month ago (2014-07-03 08:32:55 UTC) #4
You're right. I just misinterpreted the change myself, and for some reason the
testing has proven it to fix a bug for some period of time. The crash is due to
the destructor though - that's for sure. Currently investigating for a proper
fix. 

On 2014/07/02 09:35:12, Wladimir Palant wrote:
>
http://codereview.adblockplus.org/5350367445385216/diff/5629499534213120/incl...
> File include/AdblockPlus/V8ValueHolder.h (right):
> 
>
http://codereview.adblockplus.org/5350367445385216/diff/5629499534213120/incl...
> include/AdblockPlus/V8ValueHolder.h:54: reset(isolate, v8::Persistent<T>());
> I don't get it, with this change the code seems to be doing effectively the
> exact same thing. Could you explain what changed? The only difference I can
see
> is a temporary v8::Persistent instance.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5