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

allowing configuring shutdown hook #372

Merged
merged 2 commits into from
Aug 27, 2024
Merged

Conversation

eitch
Copy link
Member

@eitch eitch commented Aug 2, 2024

The shutdown hook is hooked into the JVM and is executed when the JVM stops.

This can be beneficial when the application is not managing pi4j's life cycle, but when an application wants to properly manage the life cycle, then this can be detrimental to the shutdown sequence of the application.

This change allows such an application to disable this feature.

The shutdown hook is still enabled by default, a future version might decide to disable it by default.

The shutdown hook is hooked into the JVM and is executed when the JVM stops.

This can be beneficial when the application is not managing pi4j's life cycle, but when an application wants to properly manage the life cycle, then this can be detrimental to the shutdown sequence of the application.

This change allows such an application to disable this feature.

The shutdown hook is still enabled by default, a future version might decide to disable it by default.
@eitch eitch requested a review from FDelporte August 2, 2024 12:32
@eitch eitch self-assigned this Aug 2, 2024
@eitch eitch added the enhancement New feature or request label Aug 2, 2024
@eitch
Copy link
Member Author

eitch commented Aug 2, 2024

@FDelporte should we disable it by default, or leave it enabled as is currently the case?

@FDelporte
Copy link
Member

@FDelporte should we disable it by default, or leave it enabled as is currently the case?

Not sure, I started doubting myself based on your remark in the ticket...

@eitch
Copy link
Member Author

eitch commented Aug 2, 2024

Well in the end, it is quite simple for those who care. Change the following line:

Context pi4j = Pi4J.newAutoContext();

to

Context pi4j = Pi4J.newContextBuilder().autoDetect().disableShutdownHook().build();

or if it was default disabled, they can enable it again:

Context pi4j = Pi4J.newContextBuilder().autoDetect().enableShutdownHook().build();

@eitch
Copy link
Member Author

eitch commented Aug 2, 2024

I personally would prefer having it opt-in / disabled by default.

@FDelporte
Copy link
Member

FDelporte commented Aug 2, 2024

That calls for some extra documentation ;-)
Will add with new release.

@eitch
Copy link
Member Author

eitch commented Aug 5, 2024

Ok, i now disabled the hook by default. After the checks passed, I'll merge, and then think we can do a 2.6.2, or do you think this warrants a 2.7.0? @FDelporte

@IAmNickNack
Copy link
Contributor

I'm a little confused 😄

Which option most closely resembles the existing behaviour?

It sounds like this decision could introduce a breaking change for existing users, but maybe I'm misreading this thread

@eitch
Copy link
Member Author

eitch commented Aug 26, 2024

I'm a little confused 😄

Which option most closely resembles the existing behaviour?

It sounds like this decision could introduce a breaking change for existing users, but maybe I'm misreading this thread

Yes, this is a breaking change. Existing behaviour can be reactivated with:

Context pi4j = Pi4J.newContextBuilder().autoDetect().enableShutdownHook().build();

We will communicate this in the release notes, when we release.

@eitch eitch merged commit ab9eacf into develop Aug 27, 2024
1 check passed
@eitch eitch deleted the feature/shutdown-hook-config branch August 27, 2024 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants