Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix issues with excess active processes #507

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Gooding706
Copy link

Hello Dave,
I fixed the issue mentioned in issue #416.

The issue was related to processes remaining in the active state even after they were no longer being utilized (i.e when the repl server was killed). to fix this I added the /yield command which simply sets the process to the ready state acting as the inverse of the /ping command. Just this alone didn't fix the whole problem as there was a little bug with the printing of the "Starting player processes..." message where a player process would be pinged and set to the active state purely to check if any processes exist. This issue was resolved by slightly altering the implementation of that functionality.

Let me know if there is anything I should change.

@daveyarwood
Copy link
Member

Hi @Gooding706, thank you for looking into this!

One issue here is that it isn't necessarily safe to change a player process's status from "active" to "ready". If a player process is active, it means the state of the MIDI sequencer and synthesizer are potentially changing and are no longer the initial "blank slate" that is expected when a player process is in the "ready" state.

@Gooding706
Copy link
Author

@daveyarwood, thanks for checking out my request, thats a good point I hadn't thought of. I would assume that adding systemActions.add(SystemAction.CLEAR) to the yield command may have some effect in putting the player into a clean state although I'm not 100% sure. Another solution would just be to shutdown the active process at the end of a repl session which has the disadvantage that we would lose a process, however, in the current implementation we already lose processes on shutdown as they end up inaccessible in the active state.

@daveyarwood
Copy link
Member

daveyarwood commented Mar 19, 2025

Shutting down or reverting the active process at the end of a REPL session is not really something we need to worry about. Player processes eventually shut themselves down after a period of inactivity, so the active process will go away on its own.

The issue I was talking about in #416 is that there seems to be a bug that causes the REPL server to put not 1, but 2 player processes into the "active" state, when the REPL server starts up.

@Gooding706
Copy link
Author

Gooding706 commented Mar 19, 2025

I must have misunderstood, the reason that is happening is due to the way StartingPlayerProcesses() behaves when there are available processes due to its use of FindAvailablePlayer(). I'll submit a commit with just that being addressed. Sorry about the confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants