Shut down APN client on send completion - fix memory leak

[shackpank]

Probably related: #47 , #44
Reopen of #54 from a different branch of the fork

The below graph is memory usage of our application, before and after applying this patch

[coveralls]

Coverage remained the same at 100.0% when pulling 8363a62 on shackpank:apn_client_shutdown into 8035461 on appfeel:master.

[flipace]

any idea when this will be merged?

[0x62]

Bumping @appfeel

[AdamZikmund]

Please merge it @appfeel

[c0d3m3nt0r]

To create a new connection per each push notification message it's not a good practice (see: https://goo.gl/gi3gud), connections between the Provider and APNS need to be persistent; if you have a scenario in which multiple push notifications are created (along with network connections) in a short period of time, the resulting amount of connections could be interpreted by Apple as a DDoS attack. Does it make sense?. Thanks!

[shackpank]

@c0d3m3nt0r Agree, although this pull request does not change that - currently, a new connection is already made per push message, it is just not closed after it's been sent. I guess we (as well as others on this thread) send enough pushes that our server process alerts on memory usage, but not enough for Apple to consider the number of connections opened a problem. If you're working with higher volumes I suspect you're better off using the underlying apn library directly to ensure you get the most out of each instance/open connection and handling the per-platform differences yourself.

---

As an aside, I've got this pull request published as a scoped package, in case this helps anyone while it's not yet merged into the mainline:

"@shackpank/node-pushnotifications": "1.0.18",

[santiq]

Bumping @appfeel

[0x62]

@miqmago Please merge

[kolya182]

Server goes nuts, @appfeel please merge.