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

Commit

Permalink
[Android] Fix Crash in NotifyLayout (#2902)
Browse files Browse the repository at this point in the history 
There is a concurrent read/write operation to `RenderManager::pages_` during `CoreSideInPlatform::NotifyLayout`where MainThread is reading `RenderManager::pages_` while other thread may be writing to it.

```
std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >::__is_long() const at string:1266
 (inlined by) std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >::size() const at string:905
 (inlined by) std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >::compare(std::__ndk1::basic_string_view<char, std::__ndk1::char_traits<char> >) const at string:3455
 (inlined by) std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >::compare(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) const at string:3473
 (inlined by) bool std::__ndk1::operator< <char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) at string:3693
 (inlined by) std::__ndk1::less<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > >::operator()(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) const at __functional_base:55
 (inlined by) std::__ndk1::__map_value_compare<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*>, std::__ndk1::less<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > >, true>::operator()(std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*> const&, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) const at map:476
 (inlined by) std::__ndk1::__tree_iterator<std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*>, std::__ndk1::__tree_node<std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*>, void*>*, int> std::__ndk1::__tree<std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*>, std::__ndk1::__map_value_compare<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*>, std::__ndk1::less<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > >, true>, std::__ndk1::allocator<std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*> > >::__lower_bound<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > >(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, std::__ndk1::__tree_node<std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*>, void*>*, std::__ndk1::__tree_end_node<std::__ndk1::__tree_node_base<void*>*>*) at __tree:2476
std::__ndk1::__tree_iterator<std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*>, std::__ndk1::__tree_node<std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*>, void*>*, int> std::__ndk1::__tree<std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*>, std::__ndk1::__map_value_compare<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*>, std::__ndk1::less<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > >, true>, std::__ndk1::allocator<std::__ndk1::__value_type<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*> > >::find<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > >(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) at __tree:2405
std::__ndk1::map<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, WeexCore::RenderPageBase*, std::__ndk1::less<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > >, std::__ndk1::allocator<std::__ndk1::pair<std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const, WeexCore::RenderPageBase*> > >::find(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) at map:1262
 (inlined by) WeexCore::RenderManager::GetPage(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) at render_manager.cpp:472
WeexCore::CoreSideInPlatform::NotifyLayout(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) at core_side_in_platform.cpp:223
```
  • Loading branch information
@YorkShen @lucky-chen
YorkShen authored and lucky-chen committed Sep 16, 2019
1 parent 89a1097 commit 5e8d9ac
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 26 deletions.
52 changes: 29 additions & 23 deletions android/sdk/src/main/java/com/taobao/weex/WeexFrameRateControl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@
import android.os.Build;
import android.util.Log;
import android.view.Choreographer;
import com.taobao.weex.bridge.WXBridgeManager;
import com.taobao.weex.common.WXErrorCode;
import java.lang.ref.WeakReference;

public class WeexFrameRateControl {
private static final long VSYNC_FRAME = 1000 / 60;
private WeakReference<VSyncListener> mListener;
private final Choreographer mChoreographer;
private final Choreographer.FrameCallback mVSyncFrameCallback;
private Choreographer mChoreographer;
private Choreographer.FrameCallback mVSyncFrameCallback;
private final Runnable runnable;

public interface VSyncListener {
Expand All @@ -43,27 +44,32 @@ public interface VSyncListener {
public WeexFrameRateControl(VSyncListener listener) {
mListener = new WeakReference<>(listener);
if (Build.VERSION.SDK_INT > Build.VERSION_CODES.ICE_CREAM_SANDWICH_MR1) {
mChoreographer = Choreographer.getInstance();
mVSyncFrameCallback = new Choreographer.FrameCallback() {
@SuppressLint("NewApi")
@Override
public void doFrame(long frameTimeNanos) {
VSyncListener vSyncListener;
if (mListener != null && (vSyncListener=mListener.get()) != null) {
try {
vSyncListener.OnVSync();
mChoreographer.postFrameCallback(mVSyncFrameCallback);
}catch (UnsatisfiedLinkError e){
if(vSyncListener instanceof WXSDKInstance){
((WXSDKInstance) vSyncListener).onRenderError(
WXErrorCode.WX_DEGRAD_ERR_INSTANCE_CREATE_FAILED.getErrorCode(),
Log.getStackTraceString(e));
}
}
}
}
};
runnable = null;
WXBridgeManager.getInstance().post(new Runnable() {
@Override
public void run() {
mChoreographer = Choreographer.getInstance();
mVSyncFrameCallback = new Choreographer.FrameCallback() {
@SuppressLint("NewApi")
@Override
public void doFrame(long frameTimeNanos) {
VSyncListener vSyncListener;
if (mListener != null && (vSyncListener=mListener.get()) != null) {
try {
vSyncListener.OnVSync();
mChoreographer.postFrameCallback(mVSyncFrameCallback);
}catch (UnsatisfiedLinkError e){
if(vSyncListener instanceof WXSDKInstance){
((WXSDKInstance) vSyncListener).onRenderError(
WXErrorCode.WX_DEGRAD_ERR_INSTANCE_CREATE_FAILED.getErrorCode(),
Log.getStackTraceString(e));
}
}
}
}
};
}
});
runnable = null;
} else {
// For API 15 or lower
runnable = new Runnable() {
Expand Down
6 changes: 3 additions & 3 deletions android/sdk/src/main/java/com/taobao/weex/bridge/WXBridgeManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import android.support.annotation.Nullable;
import android.support.annotation.RestrictTo;
import android.support.annotation.RestrictTo.Scope;
import android.support.annotation.UiThread;
import android.support.annotation.WorkerThread;
import android.support.v4.util.ArrayMap;
import android.text.TextUtils;
import android.util.Log;
Expand Down Expand Up @@ -3449,15 +3449,15 @@ public void bindMeasurementToRenderObject(long ptr){
* @param instanceId
* @return
*/
@UiThread
@WorkerThread
public boolean notifyLayout(String instanceId) {
if (isSkipFrameworkInit(instanceId) || isJSFrameworkInit()) {
return mWXBridge.notifyLayout(instanceId);
}
return false;
}

@UiThread
@WorkerThread
public void forceLayout(String instanceId) {
if (isSkipFrameworkInit(instanceId) || isJSFrameworkInit()) {
mWXBridge.forceLayout(instanceId);
Expand Down

0 comments on commit 5e8d9ac

Please sign in to comment.