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.

Sign up for GitHub

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

Jump to bottom

Instability on Windows 7 #7512

Closed
colonelkrud opened this issue Oct 17, 2018 · 10 comments
Closed

Instability on Windows 7 #7512

colonelkrud opened this issue Oct 17, 2018 · 10 comments
Labels
A-Electron P1 S-Critical Prevents work, causes data loss and/or has no workaround T-Defect Z-Platform-Specific

Comments

@colonelkrud
Copy link

Description

Riot becomes unresponsive after normal use on windows 7. Creating issue on behalf of user @CacMac

Steps to reproduce

  • Launch Riot desktop
  • connect to room
  • tab out of riot
  • receive image in room
  • tab in to riot to view image
  • windows task manager calls riot unresponsive
    Possible issue with image event lazy loading and windows 7 sp1

Log: sent

Version information

  • Platform: Desktop

  • OS: Windows 7 SP1

  • Version: 0.17.0
@turt2live
Copy link
Member

Potentially maybe related: #7507

@wareya
Copy link

wareya commented Oct 17, 2018

Same here, though it seems to basically happen randomly too.

@lampholder lampholder added T-Defect Z-Platform-Specific P1 S-Critical Prevents work, causes data loss and/or has no workaround A-Electron labels Oct 17, 2018
@dbkr
Copy link
Member

dbkr commented Oct 17, 2018

I've reproduced this crash (although just the instructions above didn't do it - I'm unsure what did in the end). This is presumably a regression in Electron 3. Trying to find out more...

@colonelkrud
Copy link
Author

colonelkrud commented Oct 17, 2018 •
edited
Loading

Unfortunately I am on a different platform entirely and can not help reproduce this issue. I submitted this bug on behalf of another user of my homeserver. He claims that the crash is most consistent when he hears a notification on riot and then tabs/switches focus back to the app. It does happen randomly but seems to happen more consistently when receiving image events.

The user has an older os, Windows 7 sp1. It is possible that this is the same electron issue you mentioned but it behaves differently on the older version of Windows.

I have noticed a slight lag on my system when trying to reproduce the issue. I am running the same version of the desktop app but have Mac 10.10.5 running. It is difficult to explain and does not leave a noticeable log.. but it is like the app hesitates when switching focus to view a message.

It does seem like this might be the same issue as #7507

I can have more users submit logs when they encounter this issue if it helps.

@shkshk90 shkshk90 mentioned this issue Oct 18, 2018
Closed
@shkshk90
Copy link

Hello,
I am facing this same exact issue.
I tried to debug using VS2017, but I can't get anything useful.
There seems to be a random Null pointer dereference happening somewhere, but I get no further info about the causes.
This issue does indeed happen more consistently when there's a new notification and I switch to Riot's window.
If there's anything I can submit, I am willing to help.

@dbkr
Copy link
Member

dbkr commented Oct 18, 2018

I've finally managed to get the electron debug symbols and track this down: electron is NPEing when we try to dismiss a notification.

@dbkr
Copy link
Member

dbkr commented Oct 18, 2018

Upstream bug: electron/electron#15251

@dbkr dbkr mentioned this issue Oct 18, 2018
Merged
@dbkr dbkr closed this as completed in 231ca25 Oct 18, 2018
dbkr added a commit that referenced this issue Oct 18, 2018
@dbkr
Don't set tags on notifications
2854c09 
They are to suppress notifications that don't want to be shown in
addition to each other. This makes no sense for our notifications:
they're each for independent messages. Also settings tags on
notifications makes electron crash on windows when you close the
notif, as per #7512
@dbkr dbkr mentioned this issue Oct 18, 2018
Merged
@dbkr
Copy link
Member

dbkr commented Oct 18, 2018

Updated version with workaround now available as v0.17.1

bwindels pushed a commit that referenced this issue Oct 19, 2018
@dbkr @bwindels
Don't set tags on notifications
d0fcf3c 
They are to suppress notifications that don't want to be shown in
addition to each other. This makes no sense for our notifications:
they're each for independent messages. Also settings tags on
notifications makes electron crash on windows when you close the
notif, as per #7512
@linuxmail
Copy link

Hi,

my colleague had also massive problems with 0.17 and reverted to an older (0.16.5) version back. He tried now the 0.17.1 version and instead of just a crashing Riot, the whole Windows7 had a problem, because of a massive ram usage with more than 5GB from Riot.
Sounds like a memory leak.

@colonelkrud
Copy link
Author

Hi,

my colleague had also massive problems with 0.17 and reverted to an older (0.16.5) version back. He tried now the 0.17.1 version and instead of just a crashing Riot, the whole Windows7 had a problem, because of a massive ram usage with more than 5GB from Riot.
Sounds like a memory leak.

You should open a new issue for 0.17.1 about the memory leak and post logs. It is likely a separate issue as this issue has been resolved. I mean it could be a result of this fix, I don’t know. Really should be a new issue though as this one is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Electron P1 S-Critical Prevents work, causes data loss and/or has no workaround T-Defect Z-Platform-Specific
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@wareya @dbkr @turt2live @linuxmail @lampholder @shkshk90 @colonelkrud