Skip to content
This repository has been archived by the owner on Jun 3, 2021. It is now read-only.

Commit

Permalink
Fix leaks crashes 0927 (#1566)
Browse files Browse the repository at this point in the history
* [iOS] Fix animation module crash if node ref is invalid.

* [iOS] Fix core text object leaks.

* [Core] Fix events_ object leak of render object.

* [iOS] Fix severe memory leak of data binding expression objects.

* [iOS] Fix JSValue toDictionary returns dictionary with retain-cycle and cause memory leak.

* [iOS] No task committed to bridge thread if we do not log to websocket.

* [iOS] Protect for nil NSString*.
  • Loading branch information
wqyfavor authored and cxfeng1 committed Sep 24, 2018
1 parent 19464fa commit d069b12
Show file tree
Hide file tree
Showing 10 changed files with 156 additions and 52 deletions.
49 changes: 46 additions & 3 deletions ios/sdk/WeexSDK/Sources/Bridge/WXBridgeContext.m
Original file line number Diff line number Diff line change
Expand Up @@ -467,12 +467,55 @@ - (void)createInstance:(NSString *)instanceIdString
}
}

NSDictionary* envDic = [instanceContextEnvironment toDictionary];
sdkInstance.createInstanceContextResult = [NSString stringWithFormat:@"%@", [envDic allKeys]];
JSContextRef contextRef = instanceContextEnvironment.context.JSGlobalContextRef;
WXAssert([instanceContextEnvironment isObject], @"Invalid instance context.");
JSObjectRef instanceContextObjectRef = JSValueToObject(contextRef, instanceContextEnvironment.JSValueRef, NULL);
JSPropertyNameArrayRef allKeyRefs = JSObjectCopyPropertyNames(contextRef, instanceContextObjectRef);
size_t keyCount = JSPropertyNameArrayGetCount(allKeyRefs);

BOOL somethingWrong = NO;
NSMutableArray* allKeys = [[NSMutableArray alloc] initWithCapacity:keyCount];
for (size_t i = 0; i < keyCount; i ++) {
JSStringRef nameRef = JSPropertyNameArrayGetNameAtIndex(allKeyRefs, i);
size_t len = JSStringGetMaximumUTF8CStringSize(nameRef);
if (len > 1024) {
somethingWrong = YES;
break;
}
char* buf = (char*)malloc(len + 5);
if (buf == NULL) {
somethingWrong = YES;
break;
}
bzero(buf, len + 5);
if (JSStringGetUTF8CString(nameRef, buf, len + 5) > 0) {
NSString* keyString = [NSString stringWithUTF8String:buf];
if ([keyString length] == 0) {
somethingWrong = YES;
free(buf);
break;
}
[allKeys addObject:keyString];
}
else {
somethingWrong = YES;
free(buf);
break;
}
free(buf);
}
JSPropertyNameArrayRelease(allKeyRefs);

if (somethingWrong) {
// [instanceContextEnvironment toDictionary] may contain retain-cycle.
allKeys = (NSMutableArray*)[[instanceContextEnvironment toDictionary] allKeys];
}

sdkInstance.createInstanceContextResult = [NSString stringWithFormat:@"%@", allKeys];
JSGlobalContextRef instanceContextRef = sdkInstance.instanceJavaScriptContext.javaScriptContext.JSGlobalContextRef;
JSObjectRef instanceGlobalObject = JSContextGetGlobalObject(instanceContextRef);

for (NSString * key in [envDic allKeys]) {
for (NSString * key in allKeys) {
JSStringRef propertyName = JSStringCreateWithUTF8CString([key cStringUsingEncoding:NSUTF8StringEncoding]);
if ([key isEqualToString:@"Vue"]) {
JSObjectSetPrototype(instanceContextRef, JSValueToObject(instanceContextRef, [instanceContextEnvironment valueForProperty:key].JSValueRef, NULL), JSObjectGetPrototype(instanceContextRef, instanceGlobalObject));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ - (void)_repeat:(NSArray *)repeatData inData:(NSDictionary *)data
}
}];

// set displaty:none to the redundant components;
// set display:none to the redundant components;
NSUInteger i = startIndex + repeatData.count;
while (i < self.supercomponent.subcomponents.count) {
WXComponent *component = self.supercomponent.subcomponents[i];
Expand Down Expand Up @@ -310,6 +310,10 @@ - (void)_storeBindings:(NSDictionary *)stylesOrAttributesOrEvents type:(WXDataBi
{
WXAssertComponentThread();

if (_bindingExpressions == nullptr) {
_bindingExpressions = new std::vector<WXJSExpression *>();
}

NSMutableDictionary *bindingMap;
switch (type) {
case WXDataBindingTypeProp:
Expand All @@ -334,6 +338,7 @@ - (void)_storeBindings:(NSDictionary *)stylesOrAttributesOrEvents type:(WXDataBi
NSString *bindingExpression = binding[WXBindingIdentify];
WXJSASTParser *parser = [WXJSASTParser parserWithScript:bindingExpression];
WXJSExpression *expression = [parser parseExpression];
self->_bindingExpressions->push_back(expression);
WXDataBindingBlock block = [self bindingBlockWithExpression:expression];
bindingMap[name] = block;
} else if ([binding isKindOfClass:[NSArray class]]) {
Expand All @@ -346,6 +351,7 @@ - (void)_storeBindings:(NSDictionary *)stylesOrAttributesOrEvents type:(WXDataBi
NSString *bindingExpression = bindingInArray[WXBindingIdentify];
WXJSASTParser *parser = [WXJSASTParser parserWithScript:bindingExpression];
WXJSExpression *expression = [parser parseExpression];
self->_bindingExpressions->push_back(expression);
WXDataBindingBlock block = [self bindingBlockWithExpression:expression];
bindingBlocksForIndex[@(idx)] = block;
}
Expand All @@ -372,17 +378,19 @@ - (void)_storeBindings:(NSDictionary *)stylesOrAttributesOrEvents type:(WXDataBi

if (type == WXDataBindingTypeAttributes) {
if ([WXBindingOnceIdentify isEqualToString:name]) {
_dataBindOnce = [WXConvert BOOL:binding];
self->_dataBindOnce = [WXConvert BOOL:binding];
} else if ([WXBindingMatchIdentify isEqualToString:name]) {
WXJSASTParser *parser = [WXJSASTParser parserWithScript:binding];
WXJSExpression *expression = [parser parseExpression];
_bindingMatch = [self bindingBlockWithExpression:expression];
self->_bindingExpressions->push_back(expression);
self->_bindingMatch = [self bindingBlockWithExpression:expression];
} else if ([WXBindingRepeatIdentify isEqualToString:name]) {
WXJSASTParser *parser = [WXJSASTParser parserWithScript:binding[WXBindingRepeatExprIdentify]];
WXJSExpression *expression = [parser parseExpression];
_bindingRepeat = [self bindingBlockWithExpression:expression];
_repeatIndexIdentify = binding[WXBindingRepeatIndexIdentify];
_repeatLabelIdentify = binding[WXBindingRepeatLabelIdentify];
self->_bindingExpressions->push_back(expression);
self->_bindingRepeat = [self bindingBlockWithExpression:expression];
self->_repeatIndexIdentify = binding[WXBindingRepeatIndexIdentify];
self->_repeatLabelIdentify = binding[WXBindingRepeatLabelIdentify];
}
}
}];
Expand Down
52 changes: 42 additions & 10 deletions ios/sdk/WeexSDK/Sources/Component/RecycleList/WXJSASTParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
#include <string>
#include <vector>

#define WXJSObjectFree(obj) \
if (obj) delete obj;

typedef enum : NSUInteger {
WXJSExpressionTypeUnary,
WXJSExpressionTypeBinary,
Expand Down Expand Up @@ -57,37 +60,66 @@ struct WXJSIdentifier : WXJSExpression {
};

struct WXJSMemberExpression : WXJSExpression {
WXJSExpression *object;
WXJSExpression *property;
virtual ~WXJSMemberExpression() {
WXJSObjectFree(object);
WXJSObjectFree(property);
}
WXJSExpression *object = nullptr;
WXJSExpression *property = nullptr;
bool computed;
};

struct WXJSArrayExpression : WXJSExpression {
virtual ~WXJSArrayExpression() {
for (WXJSExpression * expr : expressions) {
delete expr;
}
}
std::vector<WXJSExpression *> expressions;
};

struct WXJSUnaryExpression : WXJSExpression {
virtual ~WXJSUnaryExpression() {
WXJSObjectFree(argument);
}

std::string operator_;
bool prefix;
WXJSExpression *argument;
WXJSExpression *argument = nullptr;
};

struct WXJSBinaryExpression : WXJSExpression {
virtual ~WXJSBinaryExpression() {
WXJSObjectFree(left);
WXJSObjectFree(right);
}

std::string operator_;
WXJSExpression *left;
WXJSExpression *right;
WXJSExpression *left = nullptr;
WXJSExpression *right = nullptr;
};

struct WXJSLogicalExpression : WXJSExpression {
virtual ~WXJSLogicalExpression() {
WXJSObjectFree(left);
WXJSObjectFree(right);
}

std::string operator_;
WXJSExpression *left;
WXJSExpression *right;
WXJSExpression *left = nullptr;
WXJSExpression *right = nullptr;
};

struct WXJSConditionalExpression : WXJSExpression {
WXJSExpression *test;
WXJSExpression *alternate;
WXJSExpression *consequent;
virtual ~WXJSConditionalExpression() {
WXJSObjectFree(test);
WXJSObjectFree(alternate);
WXJSObjectFree(consequent);
}

WXJSExpression *test = nullptr;
WXJSExpression *alternate = nullptr;
WXJSExpression *consequent = nullptr;
};


Expand Down
18 changes: 9 additions & 9 deletions ios/sdk/WeexSDK/Sources/Component/RecycleList/WXJSASTParser.mm
Original file line number Diff line number Diff line change
Expand Up @@ -174,18 +174,18 @@ - (instancetype)initWithScript:(NSString *)script
return self;
}

- (void)dealloc
{
for (WXJSToken* token : _tokens) {
WXJSObjectFree(token);
}
}

- (WXJSToken *)nextToken
{
WXJSToken *token = _lookahead;

WXJSToken *next = [self lex];

_lookahead = next;

if (next->type != WXJSTokenTypeEOF) {
_tokens.push_back(token);
}

_lookahead = [self lex]; // next
_tokens.push_back(_lookahead);
return token;
}

Expand Down
9 changes: 9 additions & 0 deletions ios/sdk/WeexSDK/Sources/Component/WXComponent_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@
#import "WXConvert.h"
#import "WXTransform.h"
#import "WXTransition.h"

#ifdef __cplusplus
#import "WXJSASTParser.h"
#include <vector>
#endif // __cplusplus

@class WXTouchGestureRecognizer;
@class WXThreadSafeCounter;

Expand Down Expand Up @@ -151,6 +157,9 @@ typedef id (^WXDataBindingBlock)(NSDictionary *data, BOOL *needUpdate);
NSMutableDictionary<NSString *, WXDataBindingBlock> *_bindingAttributes;
NSMutableDictionary<NSString *, WXDataBindingBlock> *_bindingStyles;
NSMutableDictionary<NSString *, WXDataBindingBlock> *_bindingEvents;
#ifdef __cplusplus
std::vector<WXJSExpression *> *_bindingExpressions;
#endif // __cplusplus

NSMutableDictionary<NSString *, NSArray *> *_eventParameters;
}
Expand Down
30 changes: 11 additions & 19 deletions ios/sdk/WeexSDK/Sources/Component/WXTextComponent.mm
Original file line number Diff line number Diff line change
Expand Up @@ -765,34 +765,28 @@ - (void)drawTextWithContext:(CGContextRef)context bounds:(CGRect)bounds padding:
CGContextScaleCTM(context, 1.0, -1.0);

NSAttributedString * attributedStringCopy = [self ctAttributedString];
//add path
CGPathRef cgPath = NULL;
cgPath = CGPathCreateWithRect(textFrame, NULL);
CTFrameRef _coreTextFrameRef = NULL;
if (_coreTextFrameRef) {
CFRelease(_coreTextFrameRef);
_coreTextFrameRef = NULL;
}
if(!attributedStringCopy) {
if (!attributedStringCopy) {
return;
}
//add path
CGPathRef cgPath = CGPathCreateWithRect(textFrame, NULL);
CTFramesetterRef ctframesetterRef = CTFramesetterCreateWithAttributedString((__bridge CFAttributedStringRef)(attributedStringCopy));
_coreTextFrameRef = CTFramesetterCreateFrame(ctframesetterRef, CFRangeMake(0, attributedStringCopy.length), cgPath, NULL);
CFArrayRef ctLines = NULL;
if (NULL == _coreTextFrameRef) {
CTFrameRef coreTextFrameRef = CTFramesetterCreateFrame(ctframesetterRef, CFRangeMake(0, attributedStringCopy.length), cgPath, NULL);
if (NULL == coreTextFrameRef) {
// try to protect crash from frame is NULL
CFRelease(ctframesetterRef);
CGPathRelease(cgPath);
return;
}
CFRelease(ctframesetterRef);
ctframesetterRef = NULL;
ctLines = CTFrameGetLines(_coreTextFrameRef);
CFArrayRef ctLines = CTFrameGetLines(coreTextFrameRef);
CFIndex lineCount = CFArrayGetCount(ctLines);
NSMutableArray * mutableLines = [NSMutableArray new];
CGPoint lineOrigins[lineCount];
NSUInteger rowCount = 0;
BOOL needTruncation = NO;
CTLineRef ctTruncatedLine = NULL;
CTFrameGetLineOrigins(_coreTextFrameRef, CFRangeMake(0, 0), lineOrigins);
CTFrameGetLineOrigins(coreTextFrameRef, CFRangeMake(0, 0), lineOrigins);

CGFloat fixDescent = 0;
if (lineCount > 0 && _lineHeight && WX_SYS_VERSION_LESS_THAN(@"10.0")) {
Expand Down Expand Up @@ -844,16 +838,14 @@ - (void)drawTextWithContext:(CGContextRef)context bounds:(CGRect)bounds padding:
ctTruncatedLine = NULL;
continue;
}
}else {
} else {
[self drawTextWithRuns:runs context:context lineOrigin:lineOrigin];
}
}

[mutableLines removeAllObjects];
CGPathRelease(cgPath);
CFRelease(_coreTextFrameRef);
_coreTextFrameRef = NULL;
cgPath = NULL;
CFRelease(coreTextFrameRef);
CGContextRestoreGState(context);
}
}
Expand Down
9 changes: 9 additions & 0 deletions ios/sdk/WeexSDK/Sources/Model/WXComponent.mm
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,15 @@ - (void)dealloc
[self _removeAllEvents];
}
}

if (_bindingExpressions != nullptr) {
for (WXJSExpression* expr : *_bindingExpressions) {
if (expr != nullptr) {
delete expr;
}
}
delete _bindingExpressions;
}

pthread_mutex_destroy(&_propertyMutex);
pthread_mutexattr_destroy(&_propertMutexAttr);
Expand Down
3 changes: 3 additions & 0 deletions ios/sdk/WeexSDK/Sources/Module/WXAnimationModule.m
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ - (void)transition:(NSString *)nodeRef args:(NSDictionary *)args callback:(WXMod
_isAnimationedSuccess = YES;
WXPerformBlockOnComponentThread(^{
NSArray *stringArray = [nodeRef componentsSeparatedByString:@"@"];
if ([stringArray count] == 0) {
return;
}
WXComponent *targetComponent = [self.weexInstance componentForRef:stringArray[0]];
if (!targetComponent) {
if (callback) {
Expand Down
Loading

0 comments on commit d069b12

Please sign in to comment.