Memory Leak?

[takuhii]

No sure what's happening here, but I have Terminal Notifier running in 5 Terminal windows, but it is consuming GBs of memory, so much so my Macbook stops working...

[julienXX]

Which version do you use?

[takuhii]

It updated over home brew this morning, but I believe it is version 1.7.0

[julienXX]

Could you try this version https://github.com/julienXX/terminal-notifier/releases/tag/1.7.1 and tell me if it's better? (Not yet available via homebrew)

[takuhii]

ok, I'll give it a whirl and keep you posted

Thank you :)

[julienXX]

I managed to reproduce the issue when using -reply and no action is taken for example, this keeps terminal-notifier running and eating RAM.

[frdmn]

Same problem over here. 15 GB physical memory + 30 GB swap:

[julienXX]

@takuhii @frdmn what did you do with terminal-notifier in order to get this? Launch notifications manually, via a script... ?

[takuhii]

You're 1.7.1 update did the trick @julienXX I've had no more "leaks". The only thing I had to do after I applied it was reboot my Mac... I am invoking Terminal Notifier via a Script BTW ;)

[julienXX]

@frdmn do you have the issue under 1.7.1?

[frdmn]

@julienXX I was using the homebrew / Ruby wrapper of the binary. How can I extract just the terminal-notifier binary out of this .app bundle?

When I just copy it from Contents/MacOS/terminal-notifier I get the following error message:

2016-10-05 15:52:58.476 terminal-notifier[18802:3271692] No Info.plist file in application bundle or no NSPrincipalClass in the Info.plist file, exiting

[julienXX]

@frdmn 1.7.1 is available via homebrew or you can download the executable here https://github.com/julienXX/terminal-notifier/releases/tag/1.7.1

[frdmn]

@julienXX Ah great, just got the update in brew. I'm not sure how to reproduce the problem precisely, but for now none of the processes are stuck and memory looks good as well. 👍

I'll let you know if the problem still occurs with 1.7.1

[julienXX]

Great, I close the issue, feel free to re-open it if needed.

[mikaelbr]

Could this still be a problem? https://github.com/swhamilton/jest-terminal-notifier-memory-leak

cc @swhamilton

[killianbrackey]

I am having the same issue that @mikaelbr is seeing using Jest with terminal-notifier 1.7.1 updated via homebrew.
Cloud app video of the the leak: https://cl.ly/1H2I2X1T123d

[alloy]

@julienXX The original code was written with the idea that the app would always exit asap after delivery. I see that nowadays there’s the possibility for the app to remain running when waitForResponse is set and then checks if the notification is still present.

I don’t know how long it can remain ‘present’ nor have I done anything else than skim through the source, but it does look like that might be the source of the leak, as it continuously generates autoreleased strings which will never get released (until the app exits). I suggest you create the string with hash once outside of the while loop and keep an eye on other objects being created in situations where the app might be running for a long time.

[mikaelbr]

Shouldn't waitForResponse here respect the timeout?

[julienXX]

I don't yet have a fix but I'm working on it. The code for the reply option was merged from https://github.com/vjeantet/alerter and it seems the leak was already there.

[mikaelbr]

@julienXX Is this still being worked on?

[julienXX]

@mikaelbr yes it is but I'm having a hard time figuring out the proper fix.

[mikaelbr]

I see. I don't have any ObjC experience, so not much help there I'm afraid. But looks to me like the main source is the endless loop as alloy suggests. Also, are we sure checking in deliveredNotifications is the way to do it? Is that also a list of all notifications found in the notification list (not only as active notification?). In that case it would essentially be running forever. Which I think gives little value?

As for optimizing the loop it self, it looks like the entire

[NSString stringWithFormat:@"%ld", self.hash]

can be moved to above the do-while, also as @alloy says.

Looks like maybe the current code is from http://stackoverflow.com/questions/21110714/mac-os-x-nsusernotificationcenter-notification-get-dismiss-event-callback but there another identifier is used.

[alloy]

@julienXX If you have any questions or want to pair, let me know.

[julienXX]

Thanks @mikaelbr and @alloy! I already moved the NSString above the do, tried to alloc and initWithFormat to avoid using an autoreleased object but that's not enough. I reduces the leak a bit it seems but it's still there.

[julienXX]

@mikaelbr @alloy tried some updates https://gist.github.com/julienXX/bb076b0b95c738217b03e14e51f23876 but still no luck. Leaking is reduced but still there. If you have some ideas to try I'm all ears.

[mikaelbr]

Could you reuse declared arrays and other values for each iteration of the loop? Now you allocate a new array in the memory for each step, no?

[julienXX]

@mikaelbr updated the gist but not better unless I misunderstood your comment.

[julienXX]

@mikaelbr @killianbrackey @frdmn @takuhii the leak has been found and fixed thanks to @alloy help. I released 1.7.2 with the fix. Soon available via homebrew too.