search

wireapp / wire-desktop

:computer: Wire for desktop
https://wire.com/download/
GNU General Public License v3.0
1.08k stars 237 forks source link

Process handle leak on Windows #3179

Open niallnsec opened 5 years ago

niallnsec commented 5 years ago

Wire version: Version 3.10.3138 Wire for web version: Wire for Web Version 2019.10.16.1539 Operating system: Windows 10 x64 Which antivirus software do you have installed: Malwarebytes Anti-Malware

What steps will reproduce the problem?

  1. Launch Wire
  2. Handles are leaked almost immediately

What is the expected result?

The main wire process closes all open handles to child processes once they have exited.

What is the actual result?

The process handles remain open, preventing the OS from reclaiming the PID and associated resources (other than memory). Presumably the process objects are still being referenced somewhere or are not being disposed of.

Please provide any additional information below. Attach a screenshot if possible.

The following is a screenshot from ProcessHacker showing the open handles. After having Wire running on a system continuously for several days the resource consumption becomes significant. After a week I found my system started to have issues with other tasks (source compilation to be specific, as this requires many processes to spawn) which is what lead me to notice this issue during investigation.

image

Currently the issue can be worked around by simply quitting and restarting wire.

ffflorian commented 5 years ago

Hi @niallnsec, this is most likely an Electron issue over which we don't really have control.

niallnsec commented 5 years ago

Hi @ffflorian thanks for taking a look. I did wonder if it might be an electron issue. There was one thing that made me a bit unsure.

I don't have a background using Node or Electron so please excuse me if this is obviously unrelated, however, based on experience in other languages I was curious about this line: https://github.com/wireapp/wire-desktop/blob/staging/electron/src/update/squirrel.ts#L72 Its one of the few pieces of code which is calling child_process.spawn and the function resolve() seems to be called from within the its own definition. Would this not cause the call stack to get progressively longer as time goes on, each time resolve() is called? And as a result keep any stack variables from being disposed such as the const spawnedProcess definition, thus leaking handles?

I'm probably way off base here, but thought I would ask just in case.

ffflorian commented 5 years ago

Hey @niallnsec, thanks for looking into the code - the LOC you are talking about is not in production yet, please check the master version of the file: https://github.com/wireapp/wire-desktop/blob/master/electron/src/update/Squirrel.ts

About calling resolve(): it is executed only once every five minutes so even if it would add to the call stack this should happen only after a long time. And once resolved, the function execution is stopped.

xtotdam commented 4 years ago

Hi guys, I am sure you are aware of this, but this issue is still really relevant: изображение

Wire was launched for several days, I was not turning my PC off, but suspended it every day. sometimes several times a day. My PC is running Win7 SP1, Wire v.3562

xtotdam commented 4 years ago

By the way, I am also using Riot IM, their desktop client is also created using Electron, but in the same conditions they don't have this problem. So maybe this is not an Electron issue