Task/anr fix zimfile reader #3937

CalebKL · 2024-07-15T15:28:13Z

This PR fixes the ANR thats being caused while trying to create the zimFileReader object. The UI was being blocked since we were creating the object on IO Thread but we are using runblocking which blocks the main thread. The solution to the problem was to suspend setZimFile and setZimFileDescriptor. Then tie the scope on lifecycleScope on the Fragments. When the zimFileReader is being created and it takes time for the initial load, the progressBar will be visible to the users and when Loading is complete, the progressBar will be Gone.
The solution doesn't block the Main Thread and instead switches the background work to a different thread(IO)
Removed the unnecessary creation of ZimFileReader when opening notes on the Notes screen. Refactored AddNoteDialog to use database values for performing note-related operations (e.g., view, edit, delete notes) instead of setting and using ZimFileReader.
Improved the tab closing and restoring process. Previously, when the user closed all tabs, the ZimFileReader was set to null. If the user restored the tabs, a new ZimFileReader was created, which was a resource-intensive operation, especially for large ZIM files.
Now, the ZimFileReader is not set to null while the "restore tab" snackbar is visible, allowing users to restore tabs without recreating the ZimFileReader.
Once the snackbar is dismissed, the ZimFileReader is set to null to free up resources since it is no longer required.
Made the convertToBookOnDisk method suspend, as it creates the ZimFileReader object to convert the files into Books.

By doing the above changes now our UI is working flawlessly as there is no extra ZilFileReader creation happens. See the below video.

screen-20250121-171439.mp4

MohitMaliFtechiz

@CalebKL Thanks for the PR and your hard work. But there are some edge cases and crashes scenarios introduced we should fix them.

There is a crash when I frequently changed the zim files. Also, in UI there is old zim files loading when I am switching to other zim file which is bigger in size. I guess because of this, this crash is happening.

2024-07-16 14:20:19.873 12535-12609 libc                    org.kiwix.kiwixmobile                A  Fatal signal 11 (SIGSEGV), code 1, fault addr 0x0 in tid 12609 (ThreadPoolForeg)
2024-07-16 14:20:19.875 12535-12602 ZimFileReader           org.kiwix.kiwixmobile                E  Could not get Item for url = https://kiwix.app/I/Flag_of_Greece.svg.png.webp 
                                                                                                     original exception = java.lang.Exception: Cannot find entry
2024-07-16 14:20:19.876 12535-12602 ZimFileReader           org.kiwix.kiwixmobile                D  getting mimetype for https://kiwix.app/I/Flag_of_Greece.svg.png.webp = null
2024-07-16 14:20:19.877 12535-12821 ZimFileReader           org.kiwix.kiwixmobile                E  Could not get Item for url = https://kiwix.app/I/Political_slogan_by_Red_Guards_on_the_campus_of_Fudan_University_1976.jpg.webp 
                                                                                                     original exception = java.lang.Exception: Cannot find entry
2024-07-16 14:20:19.898 12535-15737 ZimFileReader           org.kiwix.kiwixmobile                E  Could not get Item for url = https://kiwix.app/I/Flag_of_Greece.svg.png.webp 
                                                                                                     original exception = java.lang.Exception: Cannot find entry
2024-07-16 14:20:19.904 12535-12535 AppCompatViewInflater   org.kiwix.kiwixmobile                I  app:theme is now deprecated. Please move to using android:theme instead.
2024-07-16 14:20:19.911 12535-12535 CubicBezierInterpolator org.kiwix.kiwixmobile                D  CubicBezierInterpolator  mControlPoint1x = 0.23, mControlPoint1y = 0.06, mControlPoint2x = 0.09, mControlPoint2y = 0.97
2024-07-16 14:20:19.912 12535-12535 CubicBezierInterpolator org.kiwix.kiwixmobile                D  CubicBezierInterpolator  mControlPoint1x = 0.6, mControlPoint1y = 0.9, mControlPoint2x = 0.8, mControlPoint2y = 1.0
2024-07-16 14:20:19.913 12535-12535 CubicBezierInterpolator org.kiwix.kiwixmobile                D  CubicBezierInterpolator  mControlPoint1x = 0.23, mControlPoint1y = 0.06, mControlPoint2x = 0.09, mControlPoint2y = 0.97
2024-07-16 14:20:19.913 12535-12535 CubicBezierInterpolator org.kiwix.kiwixmobile                D  CubicBezierInterpolator  mControlPoint1x = 0.6, mControlPoint1y = 0.9, mControlPoint2x = 0.8, mControlPoint2y = 1.0
2024-07-16 14:20:20.129 17250-17250 DEBUG                   pid-17250                            A  pid: 12535, tid: 12609, name: ThreadPoolForeg  >>> org.kiwix.kiwixmobile <<<
2024-07-16 14:20:20.132 17250-17250 DEBUG                   pid-17250                            A      #00 pc 0000000000010630  /data/app/org.kiwix.kiwixmobile-G5GGa4VbPmK5Z5zSGKhzQw==/base.apk (offset 0x71f000)
2024-07-16 14:20:20.132 17250-17250 DEBUG                   pid-17250                            A      #01 pc 000000000005be38  /data/app/org.kiwix.kiwixmobile-G5GGa4VbPmK5Z5zSGKhzQw==/oat/arm64/base.odex (offset 0x46000)

core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderContainer.kt

app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt

core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt

.../src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt

CalebKL · 2024-07-18T10:21:31Z

@MohitMaliFtechiz When I switch to different zimFiles, I get this crash after numerous attempts.
I am not sure if its the same crash you were referring to earlier.

Cmdline: org.kiwix.kiwixmobile
2024-07-18 12:44:02.286  5611-5611  DEBUG                   pid-5611                             A  pid: 5387, tid: 5606, name: DefaultDispatch  >>> org.kiwix.kiwixmobile <<<
2024-07-18 12:44:02.286  5611-5611  DEBUG                   pid-5611                             A        #00 pc 0000000000010630  /data/app/~~2cz1tgEo0aQgZshuQoQHsg==/org.kiwix.kiwixmobile-ZpQbVVYb8qkV5KrowJ_Pfg==/base.apk!libzim_wrapper.so (offset 0x1b2a000) (Java_org_kiwix_libzim_Archive_getEntryByPath__Ljava_lang_String_2+120) (BuildId: bb96395f5068f1755a93f5daa08b7a934333f07e)
2024-07-18 12:44:02.286  5611-5611  DEBUG                   pid-5611                             A        #02 pc 00000000020ad728  /memfd:jit-cache (deleted) (offset 0x2000000) (org.kiwix.kiwixmobile.core.reader.ZimFileReader.getItem+376)
2024-07-18 12:44:02.286  5611-5611  DEBUG                   pid-5611                             A        #03 pc 000000000214569c  /memfd:jit-cache (deleted) (offset 0x2000000) (org.kiwix.kiwixmobile.core.reader.ZimFileReader.loadContent+204)
2024-07-18 12:44:02.286  5611-5611  DEBUG                   pid-5611                             A        #08 pc 0000000000008cd0  /data/data/org.kiwix.kiwixmobile/code_cache/.overlay/base.apk/classes13.dex (org.kiwix.kiwixmobile.core.reader.ZimFileReader.access$loadContent+0)

MohitMaliFtechiz · 2024-07-18T11:31:42Z

@CalebKL No it is not that crash i have uploaded the crash logs in the review comment.

Also, now on my device, it is crashing immediately when I switch to other zim files(I am using the old version of Android for testing Android 8).

Fatal signal 11 (SIGSEGV), code 1, fault addr 0x0 in tid 11992 (DefaultDispatch)
2024-07-18 16:33:50.291 12077-12077 DEBUG                   pid-12077                            A  pid: 11734, tid: 11992, name: DefaultDispatch  >>> org.kiwix.kiwixmobile <<<
2024-07-18 16:33:50.294 12077-12077 DEBUG                   pid-12077                            A      #00 pc 0000000000010630  /data/app/org.kiwix.kiwixmobile-YaobaHgoDzW2aibsfWri3g==/base.apk (offset 0x71f000)
2024-07-18 16:33:50.294 12077-12077 DEBUG                   pid-12077                            A      #01 pc 000000000005be38  /data/app/org.kiwix.kiwixmobile-YaobaHgoDzW2aibsfWri3g==/oat/arm64/base.odex (offset 0x46000)

In the CoreReaderFragment there is a restore tabs functionality also for opening the previous zim file and restoring the previous tabs, and their article opens in the webView(if we are not opening the new zim file).

Currently, it is somehow restoring the previous zim file and tabs when we open the new file. However, it should not do it according to our openPageInBookFromNavigationArguments method's logic. Can you please check this? It is the main reason for these crashes.

MohitMaliFtechiz · 2024-07-18T11:35:53Z

@MohitMaliFtechiz When I switch to different zimFiles, I get this crash after numerous attempts.

This crash is newer, on which device you are testing.

CalebKL · 2024-07-18T11:42:04Z

@CalebKL No it is not that crash i have uploaded the crash logs in the review comment.

Also, now on my device, it is crashing immediately when I switch to other zim files(I am using the old version of Android for testing Android 8).
Fatal signal 11 (SIGSEGV), code 1, fault addr 0x0 in tid 11992 (DefaultDispatch)
2024-07-18 16:33:50.291 12077-12077 DEBUG                   pid-12077                            A  pid: 11734, tid: 11992, name: DefaultDispatch  >>> org.kiwix.kiwixmobile <<<
2024-07-18 16:33:50.294 12077-12077 DEBUG                   pid-12077                            A      #00 pc 0000000000010630  /data/app/org.kiwix.kiwixmobile-YaobaHgoDzW2aibsfWri3g==/base.apk (offset 0x71f000)
2024-07-18 16:33:50.294 12077-12077 DEBUG                   pid-12077                            A      #01 pc 000000000005be38  /data/app/org.kiwix.kiwixmobile-YaobaHgoDzW2aibsfWri3g==/oat/arm64/base.odex (offset 0x46000)
In the CoreReaderFragment there is a restore tabs functionality also for opening the previous zim file and restoring the previous tabs, and their article opens in the webView(if we are not opening the new zim file).

Currently, it is somehow restoring the previous zim file and tabs when we open the new file. However, it should not do it according to our openPageInBookFromNavigationArguments method's logic. Can you please check this? It is the main reason for these crashes.

let me look into this as well

CalebKL · 2024-07-18T11:42:27Z

@MohitMaliFtechiz When I switch to different zimFiles, I get this crash after numerous attempts.

This crash is newer, on which device you are testing.

I got this crash while testing on my emulator, Pixel 7 pro API34 and also on Samsung S22+

MohitMaliFtechiz · 2024-07-18T12:12:28Z

@CalebKL Thanks, it is a native crash in libzim, but currently, our PR has many bugs and workarounds that can lead to this issue, so probably first we have to fix these things. @mgautierfr can have a better idea about this error when it occurs. Might be the data is corrupted at some point due to these scenarios.

kelson42 · 2024-07-26T10:53:34Z

Any news here?

CalebKL · 2024-07-26T10:59:14Z

Any news here?

There are some fixes that I am working on from @MohitMaliFtechiz reviews. I will complete it ASAP, although this PR will need to wait until the native error(3956) causing the crash is fixed.

MohitMaliFtechiz · 2024-11-22T11:22:42Z

@CalebKL I have found some serious issues in our implementation, especially when working with Coroutines(in a multithreading environment). With the main thread, there are no issues because the implementation was made according to the main thread. But now we are moving to use the Coroutines in our code. The issue we are facing in this PR is related to #4104 which I have already fixed in #4105.

In the meantime, we have also improved our code. So now, we have a better coroutine implementation to create an archive object on the IO thread. Also, we have improved the UI side to properly show users the progress when the archive object is in the creation process: #4074.

So now, you can start working on my review comments.

kelson42 · 2024-12-10T05:37:23Z

@MohitMaliFtechiz Can you please complete PR

MohitMaliFtechiz · 2024-12-10T06:04:33Z

@kelson42, @CalebKL has already started working on this issue. So let him complete this PR.

kelson42 · 2024-12-11T05:38:59Z

5 months without activity, this is too long by far, it should be completed.

CalebKL · 2024-12-11T06:38:08Z

5 months without activity, this is too long by far, it should be completed.

Completing this ASAP

kelson42 · 2025-01-18T07:17:26Z

@MohitMaliFtechiz Please complete PR

MohitMaliFtechiz · 2025-01-21T17:36:05Z

@CalebKL I have made the necessary changes, you can test and give your feedback.

kelson42 · 2025-01-26T21:17:44Z

@MohitMaliFtechiz Please rebase

…Reader

…ince the ZIM file is already set in the reader

…restoring them. * Improved the tab closing and restoring process. Previously, when the user closed all tabs, the `ZimFileReader` was set to null. If the user restored the tabs, a new `ZimFileReader` was created, which was a resource-intensive operation, especially for large ZIM files. * Now, the `ZimFileReader` is not set to null while the "restore tab" snackbar is visible, allowing users to restore tabs without recreating the ZimFileReader. * Once the snackbar is dismissed, the `ZimFileReader` is set to null to free up resources since it is no longer required.

…in note screen since now we are not creating the ZimFileReader on note screen.

…eReader object to convert the files into Books. * Refactored the `StorageObserverTest` to align with this new change.

codecov · 2025-01-27T07:30:14Z

Codecov Report

Attention: Patch coverage is 64.70588% with 12 lines in your changes missing coverage. Please review.

Project coverage is 56.73%. Comparing base (948b8cc) to head (053b6e0).
Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
.../kiwix/kiwixmobile/core/main/CoreReaderFragment.kt	50.00%	7 Missing and 3 partials ⚠️
...java/org/kiwix/kiwixmobile/core/StorageObserver.kt	80.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3937      +/-   ##
============================================
+ Coverage     56.68%   56.73%   +0.04%     
- Complexity     1527     1532       +5     
============================================
  Files           313      313              
  Lines         13490    13510      +20     
  Branches       1693     1695       +2     
============================================
+ Hits           7647     7665      +18     
- Misses         4678     4681       +3     
+ Partials       1165     1164       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

MohitMaliFtechiz · 2025-01-27T09:05:37Z

@kelson42 Done.

MohitMaliFtechiz self-requested a review July 15, 2024 17:34

MohitMaliFtechiz requested changes Jul 16, 2024

View reviewed changes

CalebKL marked this pull request as draft July 18, 2024 11:40

MohitMaliFtechiz mentioned this pull request Jul 25, 2024

Native crash in getEntryByPath(actualPath), and getData() methods. #3956

Closed

CalebKL mentioned this pull request Dec 11, 2024

FiX ANR #4136

Closed

MohitMaliFtechiz force-pushed the task/anr-fix-zimfile-reader branch 2 times, most recently from e7933dd to be27f71 Compare January 20, 2025 09:28

MohitMaliFtechiz mentioned this pull request Jan 20, 2025

Why do we need to create the ZimFileReader object and set it to the reader if we open the note for the file that is not currently opened in the reader. #4180

Closed

MohitMaliFtechiz marked this pull request as ready for review January 21, 2025 17:34

MohitMaliFtechiz requested a review from kelson42 January 21, 2025 17:35

MohitMaliDeveloper added 4 commits January 27, 2025 12:10

Suspended setZimReaderSource and switched to IO when creating ZimFile…

9d4db58

…Reader

Creating the reader in lifecycleScope

dd5a459

Refactore code to not set the ZimFileReader when restoring the tabs s…

a181198

…ince the ZIM file is already set in the reader

Added loading functionality to the notes dialog.

c989520

MohitMaliDeveloper added 5 commits January 27, 2025 12:13

Fixed: All unit test cases.

45c74af

Reverted the changes of showing the progress while opening the notes …

2f5da0e

…in note screen since now we are not creating the ZimFileReader on note screen.

Made the convertToBookOnDisk method suspend, as it creates the ZimFil…

b1c0658

…eReader object to convert the files into Books. * Refactored the `StorageObserverTest` to align with this new change.

Improved the getting bookmarks for current book.

053b6e0

MohitMaliFtechiz force-pushed the task/anr-fix-zimfile-reader branch from d3c7656 to 053b6e0 Compare January 27, 2025 06:53

kelson42 merged commit 8bd2393 into kiwix:main Jan 27, 2025
26 of 27 checks passed

MohitMaliFtechiz mentioned this pull request Jan 30, 2025

Search is not working when we close the tab and immediately open the search. #4198

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Task/anr fix zimfile reader #3937

Task/anr fix zimfile reader #3937

CalebKL commented Jul 15, 2024 •

edited by MohitMaliFtechiz

Loading

MohitMaliFtechiz left a comment

CalebKL commented Jul 18, 2024

MohitMaliFtechiz commented Jul 18, 2024

MohitMaliFtechiz commented Jul 18, 2024

CalebKL commented Jul 18, 2024

CalebKL commented Jul 18, 2024 •

edited

Loading

MohitMaliFtechiz commented Jul 18, 2024

kelson42 commented Jul 26, 2024

CalebKL commented Jul 26, 2024

MohitMaliFtechiz commented Nov 22, 2024 •

edited

Loading

kelson42 commented Dec 10, 2024

MohitMaliFtechiz commented Dec 10, 2024 •

edited

Loading

kelson42 commented Dec 11, 2024

CalebKL commented Dec 11, 2024

kelson42 commented Jan 18, 2025

MohitMaliFtechiz commented Jan 21, 2025

kelson42 commented Jan 26, 2025

codecov bot commented Jan 27, 2025 •

edited

Loading

MohitMaliFtechiz commented Jan 27, 2025

Task/anr fix zimfile reader #3937

Task/anr fix zimfile reader #3937

Conversation

CalebKL commented Jul 15, 2024 • edited by MohitMaliFtechiz Loading

MohitMaliFtechiz left a comment

Choose a reason for hiding this comment

CalebKL commented Jul 18, 2024

MohitMaliFtechiz commented Jul 18, 2024

MohitMaliFtechiz commented Jul 18, 2024

CalebKL commented Jul 18, 2024

CalebKL commented Jul 18, 2024 • edited Loading

MohitMaliFtechiz commented Jul 18, 2024

kelson42 commented Jul 26, 2024

CalebKL commented Jul 26, 2024

MohitMaliFtechiz commented Nov 22, 2024 • edited Loading

kelson42 commented Dec 10, 2024

MohitMaliFtechiz commented Dec 10, 2024 • edited Loading

kelson42 commented Dec 11, 2024

CalebKL commented Dec 11, 2024

kelson42 commented Jan 18, 2025

MohitMaliFtechiz commented Jan 21, 2025

kelson42 commented Jan 26, 2025

codecov bot commented Jan 27, 2025 • edited Loading

Codecov Report

MohitMaliFtechiz commented Jan 27, 2025

CalebKL commented Jul 15, 2024 •

edited by MohitMaliFtechiz

Loading

CalebKL commented Jul 18, 2024 •

edited

Loading

MohitMaliFtechiz commented Nov 22, 2024 •

edited

Loading

MohitMaliFtechiz commented Dec 10, 2024 •

edited

Loading

codecov bot commented Jan 27, 2025 •

edited

Loading