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

[Android] Version 2.10.8-2.12.3 crashes on Configuration button click. Partially fixed in version 2.13.10. #7974

Closed
andrzej-nov opened this issue Mar 26, 2023 · 29 comments · Fixed by #9806 or #9864
Assignees
Labels
android bug It's a bug high High priority issues

Comments

@andrzej-nov
Copy link

andrzej-nov commented Mar 26, 2023

When I click the Configuration button, Joplin app crashes with StackOverflow exception. The issue occurred both on the full notes database from the prior Joplin version (after the app update) and on the clean database (after deleting the app data).

Environment

Joplin version: 2.10.8-2.12.3
Platform: Android
OS specifics: Lenovo TB-8704F tablet, Android 8.1.0 (SDK 27)
Partially fixed in version 2.13.10 (only some Configuration sections are crashing now).

Steps to reproduce

  1. Launch Joplin
  2. Click Configuration button
  3. Select one of following sections: Appearance, Synchronization, Note History.
  4. The app crashes

Describe what you expected to happen

The Configuration section opens.

Logfile (probably obsolete in the current version)

Unfortunately, I cannot copy full stack trace from the Android app feedback preview popup. Here are some details:

Exception: java.lang.StackOverflowError
Source class: android.content.ContextWrapper
Source method: getApplicationInfo
Line number: 152
Stack trace excerpts:
com.facebook.react.bridge.JSApplicationIllegalArgumentException: error while updating property 'step' of a view managed by RNCSlider
at com.facebook.react.urlmanager.ViewManagersPropertyCache$PropSetter.updateViewProp(ViewManagerPropertyCache.java:101)
...
Caused by: java.lang.StackOverflowError: stack size 8MB
at android.content.ContextWrapper.getApplicationInfo(ContextWrapper.java:152)
...
at android.widget.ProgressBar.setProgress(ProgressBar.java:1385)
at com.reactnativecommunity.slider.ReactSliderManager$1.onProgressChanged(ReactSliderManager.java:38)
at android.widget.ProgressBar.onProgressRefresh(SeekBar.java:95)
at android.widget.ProgressBar.doRefreshProgress(ProgressBar.java:11298)
at android.widget.ProgressBar.refreshProgress(ProgressBar.java:1353)
at android.widget.ProgressBar.setProgressInternal(ProgressBar.java:1418)
at android.widget.ProgressBar.setProgress(ProgressBar.java:1385)

(The last part loops until stack overflow).

@andrzej-nov andrzej-nov added the bug It's a bug label Mar 26, 2023
@jomu78
Copy link

jomu78 commented Apr 6, 2023

I can confirm, I have the same bug

@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2023

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? If you require support or are requesting an enhancement or feature then please create a topic on the Joplin forum. This issue may be closed if no further activity occurs. You may comment on the issue and I will leave it open. Thank you for your contributions.

@github-actions github-actions bot added the stale An issue that hasn't been active for a while... label May 6, 2023
@andrzej-nov
Copy link
Author

In the current release (as there were no update depoyments yet) it is still in place, 100% reproductible, making the app completely unusable on the affected device.

@github-actions github-actions bot removed the stale An issue that hasn't been active for a while... label May 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2023

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? If you require support or are requesting an enhancement or feature then please create a topic on the Joplin forum. This issue may be closed if no further activity occurs. You may comment on the issue and I will leave it open. Thank you for your contributions.

@github-actions github-actions bot added the stale An issue that hasn't been active for a while... label Jun 6, 2023
@andrzej-nov
Copy link
Author

In the current release (as there were no update depoyments yet) it is still in place, 100% reproductible, making the app completely unusable on the affected device.

@github-actions github-actions bot removed the stale An issue that hasn't been active for a while... label Jun 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2023

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? If you require support or are requesting an enhancement or feature then please create a topic on the Joplin forum. This issue may be closed if no further activity occurs. You may comment on the issue and I will leave it open. Thank you for your contributions.

@github-actions github-actions bot added the stale An issue that hasn't been active for a while... label Jul 7, 2023
@andrzej-nov andrzej-nov changed the title [Android] Version 2.10.8 crashes on Configuration button click [Android] Version 2.10.8-2.11.31 crashes on Configuration button click Jul 7, 2023
@andrzej-nov
Copy link
Author

Update to version 2.11.31 has not fixed the bug, it is stil there.

@github-actions github-actions bot removed the stale An issue that hasn't been active for a while... label Jul 8, 2023
@andrzej-nov andrzej-nov changed the title [Android] Version 2.10.8-2.11.31 crashes on Configuration button click [Android] Version 2.10.8-2.11.32 crashes on Configuration button click Jul 14, 2023
@niklasfi
Copy link

I am also affected by this bug and as @andrzej-nov says it is 100% reproducible on my device rendering the app completely unusable on it. I have just submitted a dump through the android crash dump collector. I'm not sure you have access to the dump that way, but I thought it might be better than nothing.

Please let me know, if there is something I can do to aid in getting to the root of the problem by providing additional logs or be of help in any other way.

@github-actions
Copy link
Contributor

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? If you require support or are requesting an enhancement or feature then please create a topic on the Joplin forum. This issue may be closed if no further activity occurs. You may comment on the issue and I will leave it open. Thank you for your contributions.

@github-actions github-actions bot added the stale An issue that hasn't been active for a while... label Aug 28, 2023
@andrzej-nov
Copy link
Author

The bug is still here.

@github-actions github-actions bot removed the stale An issue that hasn't been active for a while... label Aug 29, 2023
@andrzej-nov andrzej-nov changed the title [Android] Version 2.10.8-2.11.32 crashes on Configuration button click [Android] Version 2.10.8-2.12.2 crashes on Configuration button click Sep 4, 2023
@andrzej-nov
Copy link
Author

Update to version 2.12.2 has not fixed the bug, it is stil there.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? If you require support or are requesting an enhancement or feature then please create a topic on the Joplin forum. This issue may be closed if no further activity occurs. You may comment on the issue and I will leave it open. Thank you for your contributions.

@github-actions github-actions bot added the stale An issue that hasn't been active for a while... label Oct 4, 2023
@andrzej-nov
Copy link
Author

The issue is still actual.

@github-actions github-actions bot removed the stale An issue that hasn't been active for a while... label Oct 5, 2023
Copy link
Contributor

github-actions bot commented Nov 5, 2023

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? If you require support or are requesting an enhancement or feature then please create a topic on the Joplin forum. This issue may be closed if no further activity occurs. You may comment on the issue and I will leave it open. Thank you for your contributions.

@github-actions github-actions bot added the stale An issue that hasn't been active for a while... label Nov 5, 2023
@andrzej-nov
Copy link
Author

Update to version 2.12.3 has not fixed the bug, it is stil there.

@github-actions github-actions bot removed the stale An issue that hasn't been active for a while... label Dec 7, 2023
Copy link
Contributor

github-actions bot commented Jan 7, 2024

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? If you require support or are requesting an enhancement or feature then please create a topic on the Joplin forum. This issue may be closed if no further activity occurs. You may comment on the issue and I will leave it open. Thank you for your contributions.

@github-actions github-actions bot added the stale An issue that hasn't been active for a while... label Jan 7, 2024
@andrzej-nov
Copy link
Author

The version 2.13.10 is still the laest release, so the issue is still here.

@github-actions github-actions bot removed the stale An issue that hasn't been active for a while... label Jan 8, 2024
@personalizedrefrigerator
Copy link
Collaborator

personalizedrefrigerator commented Jan 9, 2024

From the stack trace above, this looks related to the @react-native-community/slider package. I don't see an upstream bug related to a StackOverflowError though, so it could also be related to how Joplin is configuring the slider.

Information about how Joplin uses this package

We use this package to support Setting.TYPE_INT on mobile:

} else if (md.type === Setting.TYPE_INT) {
const unitLabel = md.unitLabel ? md.unitLabel(props.value) : props.value;
const minimum = 'minimum' in md ? md.minimum : 0;
const maximum = 'maximum' in md ? md.maximum : 10;
// Note: Do NOT add the minimumTrackTintColor and maximumTrackTintColor props
// on the Slider as they are buggy and can crash the app on certain devices.
// https://github.com/laurent22/joplin/issues/2733
// https://github.com/react-native-community/react-native-slider/issues/161
return (
<View key={props.settingId} style={styleSheet.settingContainer}>
<Text key="label" style={styleSheet.settingText}>
{md.label()}
</Text>
<View style={{ display: 'flex', flexDirection: 'row', alignItems: 'center', flex: 1 }}>
<Text style={styleSheet.sliderUnits}>{unitLabel}</Text>
<Slider
key="control"
style={{ flex: 1 }}
step={md.step}
minimumValue={minimum}
maximumValue={maximum}
value={props.value}
onValueChange={newValue => void props.updateSettingValue(props.settingId, newValue)}
/>
</View>
</View>
);

I've created a React Native sample with just a slider through Expo that could be tested with Expo Go. If the issue happens with this minimal example (it doesn't for me), an upstream bug report can be created (with the demo attached).

@andrzej-nov
Copy link
Author

The minimum example works without issues on my tablet.

@andrzej-nov
Copy link
Author

Just in case, the updated stack trace from the current version (it does not differ much from the initial one):

CrashStackTrace.txt

@andrzej-nov
Copy link
Author

Just guessing (I don't know React and that is my first look at the Joplin sources):

packages\lib\components\shared\config\config-shared.ts
export const updateSettingValue method

comp.setState((state: ConfigScreenState) => {
	// @react-native-community/slider (4.4.0) will emit a valueChanged event
	// when the component is mounted, even though the value hasn't changed.
	// We should ignore this, otherwise it will mark the settings as
	// unsaved.
	//
	// Upstream: https://github.com/callstack/react-native-slider/issues/395
	//
	// https://github.com/laurent22/joplin/issues/7503
	if (state.settings[key] === value) {
		logger.info('Trying to update a setting that has not changed - skipping it.', key, value);
		return {};
	}

Could the (state.settings[key] === value) check wrongly return false when the value is not actually changing (for example, due to minor rounding issues)?

@personalizedrefrigerator personalizedrefrigerator added the high High priority issues label Jan 12, 2024
personalizedrefrigerator added a commit to personalizedrefrigerator/joplin that referenced this issue Jan 29, 2024
@personalizedrefrigerator
Copy link
Collaborator

I've created a branch that allows enabling/disabling various slider-related options (e.g. whether a minimum value is provided) and released an APK.

It may be helpful to know which of these slider-related options can be enabled without the app crashing.

@andrzej-nov
Copy link
Author

andrzej-nov commented Jan 29, 2024

Great!

I've downloaded and installed https://github.com/personalizedrefrigerator/joplin/releases/download/testing--slider-props/joplin-release.apk on my problem tablet and played with different slider options.

  1. While "Show slider" = off, the application does not crash on any option combinations I tried.

  2. While "Show slider" = on and "Set include step" = off, no option combinations cause crash either.

  3. "Show slider" = on, "Set include step" = on, "Set include minimum" = off, the app does not crash on any option combinations.

  4. "Show slider" = on, "Set include maximum" = off, "Set include step" = on, "Set include minimum" = on ... CRASH. Regardless of other options, as far as I see.

  5. Trying turning on those critical options in different order. "Show slider" = on, "Set include maximum" = off, "Set include minimum" = on, "Set include step" = on ... CRASH.

  6. "Show slider" = on, "Set include maximum" = on, "Set include minimum" = on, "Set include step" = on... no crash, regardless of other options switching.

  7. "Show slider" = on, "Set include maximum" = on, "Set include step" = on, "Set include minimum" = on... no crash, regardless of other options switching.

  8. If from the combination ("Show slider" = on, "Set include maximum" = on, "Set include step" = on, "Set include minimum" = on) I click "Reload clider", or switch "Show slider" off and back on, app crashes.

So it seems that the order of setting the options is critical: "Set include maximum" must be set on before "Set include minimum" and "Set include step" are on. Reverse order (setting minimum and step both on, in any order, while maximum is still off) causes crash.

As for crashes when enabling "Show slider" or clicking "Reload slider", I suppose they use fixed order of scanning and setting options, and it does not set maximum before minimum and step.

Hope that helps.

@personalizedrefrigerator
Copy link
Collaborator

personalizedrefrigerator commented Jan 30, 2024

That was helpful!

I suspect that the issue is related to the upper limit of the slider being larger than the lower limit (related upstream code). I haven't gotten this to happen locally, but can reproduce a StackOverflow if I hardcode the lower limit to be larger than the upper limit by patching the upstream code.

Looking at this StackOverflow thread and the Android ProgressBar source code (used by react-native-slider), it looks like onProgress events shouldn't be fired by setProgress if the new progress is out-of-bounds, so perhaps the min/max values aren't being sent to Android before the first time the onProgress event is fired.

I've published another APK that patches the slider library we use to:

  1. Prevent the StackOverflow error above (caps recursion at 15-20).
    • This is just for testing -- it doesn't fix the underlying issue.
  2. Log various values. Slider logs can be copied/shared by clicking the "Pull logs" button.

@andrzej-nov
Copy link
Author

Nice! I haven't managed to crash this version.
The slider behaves a bit funny (reacts with a delay sometimes, or needs to move back and forth several times until it sets the needed value), but everything works. The current behaviour is more an annoyance than a bug.

I've recorded a couple of logs, just in case.
Log 1.txt
Log 2.txt

@personalizedrefrigerator
Copy link
Collaborator

Re-opened by 6dbfa6e

@laurent22
Copy link
Owner

Re-opened by 6dbfa6e

Is it possible to apply the patch to all versions of @react-native-community/slider? I assume that if for some reason the patch cannot be applied the build will fail and we'll know the patch needs to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment