Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ios): thread race of bundle load op in very small scenarios #4064

Merged
merged 3 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
View,
StyleSheet,
Text,
Platform,
} from '@hippy/react';

const STYLE_LOADING = 100;
Expand Down Expand Up @@ -271,8 +270,7 @@ export default class ListExample extends React.Component {
return true;
}}
bounces={true}
// horizontal ListView flag(only Android support)
horizontal={horizontal}
horizontal={horizontal} // horizontal ListView flag
style={[{ backgroundColor: '#ffffff' }, horizontal ? { height: 50 } : { flex: 1 }]}
numberOfRows={dataSource.length}
renderRow={this.getRenderRow}
Expand All @@ -297,33 +295,32 @@ export default class ListExample extends React.Component {
onScroll={this.onScroll}
scrollEventThrottle={1000} // 1s
/>
{Platform.OS === 'android'
? <View
onClick={() => this.changeDirection()}
style={{
position: 'absolute',
right: 20,
bottom: 20,
width: 67,
height: 67,
borderRadius: 30,
boxShadowOpacity: 0.6,
boxShadowRadius: 5,
boxShadowOffsetX: 3,
boxShadowOffsetY: 3,
boxShadowColor: '#4c9afa' }}>
<View style={{
width: 60,
height: 60,
borderRadius: 30,
backgroundColor: '#4c9afa',
display: 'flex',
justifyContent: 'center',
alignItems: 'center',
}}>
<View
onClick={() => this.changeDirection()}
style={{
position: 'absolute',
right: 20,
bottom: 20,
width: 67,
height: 67,
borderRadius: 30,
boxShadowOpacity: 0.6,
boxShadowRadius: 5,
boxShadowOffsetX: 3,
boxShadowOffsetY: 3,
boxShadowColor: '#4c9afa' }}>
<View style={{
width: 60,
height: 60,
borderRadius: 30,
backgroundColor: '#4c9afa',
display: 'flex',
justifyContent: 'center',
alignItems: 'center',
}}>
<Text style={{ color: 'white' }}>切换方向</Text>
</View>
</View> : null}
</View>
</View>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@
</li>
</ul>
<div
v-if="Vue.Native.Platform === 'android'"
:style="{
position: 'absolute',
right: 20,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@
</li>
</ul>
<div
v-if="Platform === 'android'"
:style="{
position: 'absolute',
right: 20,
Expand Down Expand Up @@ -114,7 +113,7 @@
</template>

<script lang="ts">
import { type ListViewEvent, Native } from '@hippy/vue-next';
import { type ListViewEvent } from '@hippy/vue-next';
import { defineComponent, ref, onMounted, type Ref } from '@vue/runtime-core';

const STYLE_LOADING = 100;
Expand Down Expand Up @@ -269,7 +268,6 @@ export default defineComponent({
list,
STYLE_LOADING,
horizontal,
Platform: Native.Platform,
onAppear,
onDelete,
onDisappear,
Expand Down
21 changes: 15 additions & 6 deletions framework/ios/base/bridge/HippyBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ @interface HippyBridge() {
/// Bundle fetch operation queue (concurrent)
@property (nonatomic, strong) NSOperationQueue *bundleQueue;
/// Record the last execute operation for adding execution dependency.
@property (atomic, strong, nullable) NSOperation *lastExecuteOperation;
@property (nonatomic, strong, nullable) NSOperation *lastExecuteOperation;

/// Cached Dimensions info,will be passed to JS Side.
@property (atomic, strong) NSDictionary *cachedDimensionsInfo;
Expand Down Expand Up @@ -591,13 +591,17 @@ - (void)beginLoadingBundle:(NSURL *)bundleURL
strongSelf.valid, script];
HippyLogError(@"%@", errMsg);
completion(bundleURL, HippyErrorWithMessage(errMsg));
strongSelf.lastExecuteOperation = nil;
@synchronized (self) {
strongSelf.lastExecuteOperation = nil;
}
return;
}
[strongSelf executeJSCode:script sourceURL:bundleURL onCompletion:^(id result, NSError *error) {
HippyLogInfo(@"End executing bundle(%s)",
HP_CSTR_NOT_NULL(bundleURL.absoluteString.lastPathComponent.UTF8String));
strongSelf.lastExecuteOperation = nil;
@synchronized (self) {
strongSelf.lastExecuteOperation = nil;
}
if (completion) {
completion(bundleURL, error);
}
Expand All @@ -624,13 +628,18 @@ - (void)beginLoadingBundle:(NSURL *)bundleURL
// Add dependency, make sure that doing fetch before execute,
// and all execution operations must be queued.
[executeOperation addDependency:fetchOperation];
if (self.lastExecuteOperation) {
[executeOperation addDependency:self.lastExecuteOperation];
@synchronized (self) {
NSOperation *lastOp = self.lastExecuteOperation;
if (lastOp) {
[executeOperation addDependency:lastOp];
}
}

// Enqueue operation
[_bundleQueue addOperations:@[fetchOperation, executeOperation] waitUntilFinished:NO];
self.lastExecuteOperation = executeOperation;
@synchronized (self) {
self.lastExecuteOperation = executeOperation;
}
}

- (void)unloadInstanceForRootView:(NSNumber *)rootTag {
Expand Down
6 changes: 3 additions & 3 deletions modules/footstone/include/footstone/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,9 @@ bool ShouldCreateLogMessage(LogSeverity severity);

#define HP_CSTR_NOT_NULL( p ) (p ? p : "")

#ifdef DEBUG
#ifdef ENABLE_HIPPY_PERFLOG
// enable perf log output when `ENABLE_HIPPY_PERFLOG` is set

// enable perf log output in debug mode only
#define TDF_PERF_LOG(format, ...) \
footstone::LogMessage::LogWithFormat(__FILE_NAME__, __LINE__, "[HP PERF] " format, \
##__VA_ARGS__)
Expand All @@ -241,4 +241,4 @@ footstone::LogMessage::LogWithFormat(__FILE_NAME__, __LINE__, "[HP PERF] " forma
#define TDF_PERF_LOG(format, ...)
#define TDF_PERF_DO_STMT_AND_LOG(STMT , format, ...)

#endif
#endif /* ENABLE_HIPPY_PERFLOG */
Loading