Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 5350367445385216: Fix the crashing (Closed)

Created:
July 1, 2014, 10:56 p.m. by Oleksandr
Modified:
July 3, 2014, 8:33 a.m.
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
July 1, 2014, 10:58 p.m. (2014-07-01 22:58:49 UTC) #1
Oleksandr
July 2, 2014, 9:20 a.m. (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 ...
July 2, 2014, 9:35 a.m. (2014-07-02 09:35:12 UTC) #3
Oleksandr
July 3, 2014, 8:32 a.m. (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.

Powered by Google App Engine
This is Rietveld