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

Freeze in scrolling message list, null parentData.layoutOffset #725

Closed
gnprice opened this issue Jun 5, 2024 · 2 comments · Fixed by #1312
Closed

Freeze in scrolling message list, null parentData.layoutOffset #725

gnprice opened this issue Jun 5, 2024 · 2 comments · Fixed by #1312
Assignees
Labels
a-msglist The message-list screen, except what's label:a-content a-sticky_header Our `sticky_header` library

Comments

@gnprice
Copy link
Member

gnprice commented Jun 5, 2024

Using the app today, I was scrolling in the message list and then it froze — stopped responding to my attempts to scroll.

I went and connected the phone to my desktop and looked at logcat. It shows the following stack trace:

Null check operator used on a null value
#0      _RenderSliverStickyHeaderListInner._findChildAtEnd (package:zulip/widgets/sticky_header.dart:719)
#1      _RenderSliverStickyHeaderListInner.performLayout (package:zulip/widgets/sticky_header.dart:740)
#2      RenderObject.layout (package:flutter/src/rendering/object.dart:2577)
#3      _RenderSliverStickyHeaderList.performLayout (package:zulip/widgets/sticky_header.dart:556)
#4      RenderObject.layout (package:flutter/src/rendering/object.dart:2577)
#5      RenderViewportBase.layoutChildSequence (package:flutter/src/rendering/viewport.dart:601)
#6      RenderViewport._attemptLayout (package:flutter/src/rendering/viewport.dart:1497)
#7      RenderViewport.performLayout (package:flutter/src/rendering/viewport.dart:1427)
#8      RenderObject._layoutWithoutResize (package:flutter/src/rendering/object.dart:2416)
#9      PipelineOwner.flushLayout (package:flutter/src/rendering/object.dart:1052)
#10     PipelineOwner.flushLayout (package:flutter/src/rendering/object.dart:1065)
#11     RendererBinding.drawFrame (package:flutter/src/rendering/binding.dart:602)
#12     WidgetsBinding.drawFrame (package:flutter/src/widgets/binding.dart:1164)
#13     RendererBinding._handlePersistentFrameCallback (package:flutter/src/rendering/binding.dart:468)
#14     SchedulerBinding._invokeFrameCallback (package:flutter/src/scheduler/binding.dart:1392)
#15     SchedulerBinding.handleDrawFrame (package:flutter/src/scheduler/binding.dart:1313)
#16     SchedulerBinding._handleDrawFrame (package:flutter/src/scheduler/binding.dart:1171)
#17     _invoke (dart:ui/hooks.dart:312)
#18     PlatformDispatcher._drawFrame (dart:ui/platform_dispatcher.dart:419)
#19     _drawFrame (dart:ui/hooks.dart:283)

Then over time after that there's this error, again and again many times:

Another exception was thrown: Instance of 'DiagnosticsProperty<void>'

There's a fresh burst of those any time I touch the screen and try to scroll, or when


The stack trace shows the exception is from our sticky_header library. The null value in question seems to be the parentData.layoutOffset below, quoting some code ending on the line that has the ! that blew up:

    RenderBox? child;
    for (child = lastChild; ; child = childBefore(child)) {
      if (child == null) {
        // Ran out of children.
        return null;
      }
      final parentData = child.parentData! as SliverMultiBoxAdaptorParentData;
      assert(parentData.layoutOffset != null);
      if (endOffset > parentData.layoutOffset! + child.size.onAxis(constraints.axis)) {

So this function is assuming that the children have their parentData all set up, including layoutOffset, but there's a child that doesn't have that. I haven't debugged further.


Further observations from the context and symptoms:

  • The app had been open a while earlier, showing the message list in question (namely "Combined feed"). Then it was in the background and I re-opened it.
  • When the freeze happened, I was scrolled up a few screenfuls in the history.
  • The visible state on the screen was a bit funny-looking: mostly full of messages as normal, but with a gray gap at the bottom.
  • The app didn't crash or completely stop. As new messages subsequently come in, the message list keeps updating to show new messages. (At some point it jumped to the bottom of the history.)
    • The visible state continues to look funny with variations on the original: mostly full of messages, but with a gray gap somewhere between them.
    • Specifically at the moment it looks like one of the recipient headers, the one for the 4th out of 5 messages currently visible, is replaced with a gray gap that's about the height the recipient header should be. (I compared with the same narrow on web to see that there's no messages missing.)
    • At another time there was a much wider gray gap, immediately after a recipient header. So perhaps a message itself was being replaced with a gray gap. … OK, and now I'm seeing that again, and indeed it's one message missing. … And now again, and it's a message missing which isn't immediately after a recipient header (it's between two messages).
  • Although it's updating in response to server events, nothing I do when I try to interact with the message list has any effect: not scrolling (as mentioned), nor touching a link, nor a recipient header, nor an image. They all just trigger another Another exception was thrown: Instance of 'DiagnosticsProperty<void>' in the log.
  • After all those observations, I touched the back button at top left of the app, and it worked immediately. Then I navigated back into "Combined feed", and it's working normally.

So my best guess for how to potentially reproduce the issue is:

  • Scroll up a few screenfuls in history.
  • Have the PerAccountState get swapped out for a new one. (To do this without waiting 10 minutes, one can fiddle with the retry logic in UpdateMachine.poll.)

(Meanwhile the "Instance of 'DiagnosticsProperty'" error messages are a bit of a mystery. Those seem like there's probably a bug in error-reporting somewhere, which would be good to fix in order to better enable debugging other bugs, like this one.)

@gnprice gnprice added a-msglist The message-list screen, except what's label:a-content a-sticky_header Our `sticky_header` library labels Jun 5, 2024
@gnprice gnprice added this to the Beta 3: Summer 2024 milestone Jun 5, 2024
@gnprice
Copy link
Member Author

gnprice commented Jul 26, 2024

Moving this to the post-launch milestone because it's only ever happened the one time as far as I know, and it's been a couple of months since then.

If we see it another time or two, or hear a user report of it, we can bump it back up in priority.

@gnprice
Copy link
Member Author

gnprice commented Jan 28, 2025

Rereading my report above in the light of having discovered #1309, I'm fairly confident now that this is another symptom of the same underlying bug. So I'll mark the same fix as fixing both of these.

@gnprice gnprice self-assigned this Jan 28, 2025
gnprice added a commit to gnprice/zulip-flutter that referenced this issue Jan 29, 2025
Fixes zulip#1309.
Fixes zulip#725.

This is necessary when scrolling back to the origin over items that
grew taller while off screen (and beyond the 250px of near-on-screen
cached items).  For example that can happen because a message was
edited, or because new messages came in that were taller than those
previously present.
gnprice added a commit to gnprice/zulip-flutter that referenced this issue Jan 31, 2025
Fixes zulip#1309.
Fixes zulip#725.

This is necessary when scrolling back to the origin over items that
grew taller while off screen (and beyond the 250px of near-on-screen
cached items).  For example that can happen because a message was
edited, or because new messages came in that were taller than those
previously present.
@gnprice gnprice closed this as completed in 66cc727 Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-msglist The message-list screen, except what's label:a-content a-sticky_header Our `sticky_header` library
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant