Adding logic to ensure teardown is called before run_forever exits, otherwise resources like the ping thread are leaked.

[QuinnDamerell]

Fixes issue #917, see my description there on how it's possible to return from run_forever without calling teardown. There might be other ways as well, so my solution here is a good way to ensure it's called.

[codecov]

Codecov Report

Patch coverage: 93.33% and project coverage change: +0.31 🎉

Comparison is base (eda6724) 83.45% compared to head (3a487eb) 83.77%.

❗ Current head 3a487eb differs from pull request most recent head 0449bd6. Consider uploading reports for the commit 0449bd6 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #918      +/-   ##
==========================================
+ Coverage   83.45%   83.77%   +0.31%     
==========================================
  Files          13       13              
  Lines        1360     1368       +8     
  Branches      257      259       +2     
==========================================
+ Hits         1135     1146      +11     
+ Misses        149      147       -2     
+ Partials       76       75       -1     

Impacted Files	Coverage Δ
websocket/_app.py	81.85% <93.33%> (+1.77%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[wormsik]

Great work @QuinnDamerell ! Do you think it solves also #612 ? From the quick look at the code I guess it should as on_close is executed inside teardown method.

[engn33r]

Excellent work identifying the specific code path that causes this issue. I will request @bubbleboy14 to give a final comment before I merge this and create a new 1.6.0 minor release, but this PR looks like a very welcome upgrade.

I tested this updated code on the two problematic code examples where on_close was not getting triggered and this solves them 👍
https://websocket-client.readthedocs.io/en/v1.5.2/threading.html#possible-issues-with-threading

[bubbleboy14]

Great job @QuinnDamerell ! These look like excellent changes, and they pass my tests.

I say go for it!

[QuinnDamerell]

@engn33r, no pressure, but do you have any timeline on when this change might happen? I really want to pull it into my PY app, because I have quite a few users who hit this.

I'm happy to help if you need any assistance in testing or whatnot to get it in!