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

feat(server/impl): add onServerStop event #2670

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DaniGP17
Copy link

@DaniGP17 DaniGP17 commented Jul 25, 2024

Goal of this PR

Added an event to handle necessary actions when the server is stopped. Currently I need to use txadmin to know when the server is going to stop with the txAdmin:events:serverShuttingDown event.

How is this PR achieving the goal

Created an event that is executed when the quit is requested and when the quit is forced using CTRL+C.

This PR applies to the following area(s)

Server

Successfully tested on

Platforms: Windows

Checklist

  • Code compiles and has been tested successfully.
  • Code explains itself well and/or is documented.
  • My commit message explains what the changes do and what they are for.
  • No extra compilation warnings are added by these changes.

@github-actions github-actions bot added the triage Needs a preliminary assessment to determine the urgency and required action label Jul 25, 2024
@Yum1x
Copy link

Yum1x commented Jul 26, 2024

IMO
this should be managed by another application (such as TxAdmin), as the way you proposed is not capable of handling when a crash occours.

@DaniGP17
Copy link
Author

DaniGP17 commented Jul 26, 2024

IMO this should be managed by another application (such as TxAdmin), as the way you proposed is not capable of handling when a crash occours.

Does txadmin handle when a crash occours?
I've crashed my server using txadmin and the event txAdmin:events:serverShuttingDown is not triggered, the same as in my way

[           resources] Stopping resource test
[    c-scripting-core] Creating script environments for test
[           resources] Started resource test
[         script:test] CRASHING SERVER

=================================================================
        Native Crash Reporting
=================================================================
Got a SIGSEGV while executing native code. This usually indicates
a fatal error in the mono runtime or one of the native libraries 
used by your application.
=================================================================

=================================================================
        Managed Stacktrace:
=================================================================
=================================================================
[ citizen-server-impl] network thread hitch warning: timer interval of 248 milliseconds
[ citizen-server-impl] sync thread hitch warning: timer interval of 252 milliseconds
[ citizen-server-impl]

@Yum1x
Copy link

Yum1x commented Jul 26, 2024

IMO this should be managed by another application (such as TxAdmin), as the way you proposed is not capable of handling when a crash occours.

Does txadmin handle when a crash occours? I've crashed my server using txadmin and the event txAdmin:events:serverShuttingDown is not triggered, the same as in my way

[           resources] Stopping resource test
[    c-scripting-core] Creating script environments for test
[           resources] Started resource test
[         script:test] CRASHING SERVER

=================================================================
        Native Crash Reporting
=================================================================
Got a SIGSEGV while executing native code. This usually indicates
a fatal error in the mono runtime or one of the native libraries 
used by your application.
=================================================================

=================================================================
        Managed Stacktrace:
=================================================================
=================================================================
[ citizen-server-impl] network thread hitch warning: timer interval of 248 milliseconds
[ citizen-server-impl] sync thread hitch warning: timer interval of 252 milliseconds
[ citizen-server-impl]

That's I said .... u wont be able to hear txAdmin:events:serverShuttingDown(or any other event like u proposed) in FxServer when a crash occurs. U should write ur own solution to handle this case or ask this feature in TxAdmin repo.

@AvarianKnight
Copy link
Contributor

AvarianKnight commented Jul 26, 2024

Added an event to handle necessary actions when the server is stopped. Currently I need to use txadmin to know when the server is going to stop with the txAdmin:events:serverShuttingDown event.

Did you actually test to see if this works? I'm not quite sure how FXServer handles shutting down but will the resources last long enough to even handle this event?

One might assume that someone would try to asynchronous tasks with this too (like saving players) which would also probably not work because the server could be shut down before the resource gets a chance to execute.

That's I said .... u wont be able to hear txAdmin:events:serverShuttingDown(or any other event like u proposed) in FxServer when a crash occurs. U should write ur own solution to handle this case or ask this feature in TxAdmin repo.

This seems like a very weird take, the server can't trigger events if its crashing so nothing can handle this. Doing handling this event when the server is in an inconsistent state doesn't sound like something that should be supported by anything anyways.

@DaniGP17
Copy link
Author

DaniGP17 commented Jul 26, 2024

Did you actually test to see if this works? I'm not quite sure how FXServer handles shutting down but will the resources last long enough to even handle this event?

Yeah, in my case I'm saving some info of the player into DB, this is a bit random because I get different times, may be the delay that take the server to close the process.

In that case TXAdmin trigger the event, wait some time and then kill the server process

@github-actions github-actions bot added invalid Requires changes before it's considered valid and can be (re)triaged and removed triage Needs a preliminary assessment to determine the urgency and required action labels Aug 21, 2024
@DaniGP17 DaniGP17 closed this Aug 21, 2024
@DaniGP17 DaniGP17 reopened this Aug 21, 2024
@DaniGP17 DaniGP17 force-pushed the master branch 2 times, most recently from 5147028 to 962be88 Compare August 21, 2024 11:09
@github-actions github-actions bot added triage Needs a preliminary assessment to determine the urgency and required action invalid Requires changes before it's considered valid and can be (re)triaged and removed invalid Requires changes before it's considered valid and can be (re)triaged triage Needs a preliminary assessment to determine the urgency and required action labels Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid Requires changes before it's considered valid and can be (re)triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants