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

MixinService lookup mechanism limits logging abstraction and causes unintentional Log4J use #569

Open
sfPlayer1 opened this issue Apr 1, 2022 · 0 comments

Comments

@sfPlayer1
Copy link

Mixin abstracts logging away to not use a particularly library, based on what the IMixinService implementation chooses to use. The IMixinService itself gets discovered through ServiceLoader and has a variety of builtin implementations bundled with Mixin's releases.

The problem is that MixinServiceAbstract as the super class of most Mixin services including MixinServiceModLauncher creates the logger in the constructor. The service loader mechanism constructs all implementations during regular use (lazily). So Mixin looks for IMixinServices, runs across MixinServiceModLauncher, instantiates it, which instantiates LoggerAdapterLog4j2, which uses and loads Log4J2.

My particular scenario doesn't really support Log4J2 in this context (config/plugins/partial class path), which leads to some garbage on stderr and potentially other unwanted state changes. The problem goes away when our IMixinService comes first, but this isn't something we can always guarantee.

For my uses it'd be sufficient to have a way to bypass the service loader based IMixinService lookup, e.g. through a system property declaring the desired impl class directly or an API to pass the instance cleanly (difficult with the current code flow). This would bypass MixinServiceModLauncher and others, working around the problem outlined above. It's probably also speed initialization up a little, the desired service impl is well known in advance.

Long term it'd be nice to change IMixinService discovery such that each service verifies its eligibility for the environment before having any impact on it, particularly not loading any classes specific to it, directly or indirectly.

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

No branches or pull requests

1 participant