Fixing crashes, code cleaning, and features!

[CodeforEvolution]

I think I got carried away... :)

[CodeforEvolution]

Hold up...one quick problem that I introduced that needs to be fixed...

[humdingerb]

@CodeForEvolution, ready to be merged? Or, what's the quick problem?

[Vidrep]

humdinger, I've been using this brach for a few days now. Not a single crash. Maybe just a couple of tweaks to the GUI are all that's needed now.

[CodeforEvolution]

Unfortunately this is still not mergeable, even though the program isn’t crashing, without that “delete”, StreamRadio will start leaking network threads and audio resources when switching between stations. I just don’t have the time to look into this further...though I think it might be BLocker related?

[humdingerb]

Can you link to the specific location(s) in the code that's the issue? Not that I can fix it, most probably, but maybe someone else can... :)

Are the leaks that severe that it impacts usage so significantly, that we shouldn't release these PRs improvements?

[CodeforEvolution]

https://github.com/CodeforEvolution/StreamRadio/blob/029dbd4c1212533b39cbe73c42f79db5507bc448/MainWindow.cpp#L321 Before my changes, the "player" variable was deleted right above this line of code, which deleted an instance of the "StreamPlayer" class (a class that handles playback for a single station). This "delete" was causing a crash for me (and it seems for you guys too) that occurred after playing and pausing any station two times. The crash had to do with BLocker when it was trying to be destructed (which StreamPlayer is based upon). But the problem is pretty simple if we don't use delete: The StreamPlayer is leaked. This is because once the associated "StationItem" instance is unmapped from it, nothing else is keeping track of the StreamPlayer, and thus its resources, including a network thread that was used for reading the audio data, is leaked.

[Vidrep]

Playing around with Cppcheck, so decided to run it on MainWindow.cpp. Result below. Hope it helps.

~/HaikuArchives/StreamRadio-crashEtClean> cppcheck --enable=all MainWindow.cpp Checking MainWindow.cpp ... [MainWindow.cpp:53]: (style) C-style pointer casting [StationListView.h:41]: (style) Class 'StationListViewItem' has a constructor with 1 argument that is not explicit. [StationListView.h:74]: (style) Class 'StationListView' has a constructor with 1 argument that is not explicit. [StationFinder.h:74]: (style) Class 'FindByCapability' has a constructor with 1 argument that is not explicit. [StationFinder.h:132]: (style) Class 'StationFinderWindow' has a constructor with 1 argument that is not explicit. Checking MainWindow.cpp: DEBUGGING... Checking MainWindow.cpp: PROFILING... Checking MainWindow.cpp: TRACING... [MainWindow.cpp:393]: (style) The function 'QuitRequested' is never used.

[jsteinaker]

Sorry about not quoting and stuff. I might have discovered what's happening here (but then again, I don't know jack about C++ so...). On the StreamPlayer class destructor, I've just commented the first line, which posts the MSG_PLAYER_STATE_CHANGED message (Stopped). If you take a look at MainWindow.cpp, when the window receives a message like that triggers the delete routine again, which in turn calls the destructor, which will try to post the message again, thus causing a loop or double-delete

This change (plus the others) is already on PR. Can someone give it a try and confirm if it's working (and correct me if I'm wrong about what I think it's happening)?

[humdingerb]

This change (plus the others) is already on PR. Can someone give it a try and confirm if it's working (and correct me if I'm wrong about what I think it's happening)?

Just to clarify, the PR you ask to try is #39, right? (Pulkomandy and I alredy commented there.)