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

ContextInitializer.autoConfig() doesn't close resource that could be leak #191

Closed
FilenkovMaxim opened this issue Oct 26, 2018 · 11 comments
Closed
Labels

Comments

@FilenkovMaxim
Copy link

FilenkovMaxim commented Oct 26, 2018

implementation 'com.github.tony19:logback-android:1.3.0-2'

Run

StrictMode.setThreadPolicy(new StrictMode.ThreadPolicy.Builder()
          .detectAll()
          .penaltyLog()
          .build());
StrictMode.setVmPolicy(new StrictMode.VmPolicy.Builder()
          .detectAll()
          .penaltyLog()
          .build());

ContextInitializer ci = new ContextInitializer(lc);
ci.autoConfig();

got the error

A resource was acquired at attached stack trace but never released. See java.io.Closeable for information on avoiding resource leaks.
    java.lang.Throwable: Explicit termination method 'close' not called
        at dalvik.system.CloseGuard.open(CloseGuard.java:180)
        at java.util.zip.ZipFile.<init>(ZipFile.java:185)
        at java.util.jar.JarFile.<init>(JarFile.java:199)
        at libcore.net.url.JarURLConnectionImpl.openJarFile(JarURLConnectionImpl.java:136)
        at libcore.net.url.JarURLConnectionImpl.findJarFile(JarURLConnectionImpl.java:125)
        at libcore.net.url.JarURLConnectionImpl.connect(JarURLConnectionImpl.java:82)
        at libcore.net.url.JarURLConnectionImpl.getInputStream(JarURLConnectionImpl.java:215)
        at ch.qos.logback.core.joran.GenericConfigurator.doConfigure(Unknown Source)
        at ch.qos.logback.classic.util.ContextInitializer.autoConfig(Unknown Source)
@FilenkovMaxim FilenkovMaxim changed the title ContextInitializer.autoConfig() doesn't close resource that could be leak ContextInitializer.autoConfig() doesn't close resource that could be leaks Oct 26, 2018
@FilenkovMaxim FilenkovMaxim changed the title ContextInitializer.autoConfig() doesn't close resource that could be leaks ContextInitializer.autoConfig() doesn't close resource that could be leak Oct 26, 2018
@tony19
Copy link
Owner

tony19 commented Oct 26, 2018

I can't reproduce the problem. Can you provide a link to a GitHub repo that exhibits this?

Note that you shouldn't be calling ContextInitializer.autoConfig() because it's already automatically called (hence its name).

@FilenkovMaxim
Copy link
Author

Did you set StrictMode?
I call it because I clean log directory after I sent logs to my server.

@tony19
Copy link
Owner

tony19 commented Oct 26, 2018

Did you set StrictMode?

Yes

I call it because I clean log directory after I sent logs to my server.

ContextInitializer.autoConfig() isn't intended to do that.

@FilenkovMaxim
Copy link
Author

Ok, I'll try to reproduce this in a blank example and will upload it to github.

@tony19
Copy link
Owner

tony19 commented Nov 13, 2018

I was able to reproduce the issue with some minor modifications to your code.

The GenericConfigurator.doConfigure() methods in logback should be closing the input streams in a finally block (a logback bug), which would resolve the leak issue. In addition, the lc.reset() in resetLogs() should be lc.stop() in your case in order to properly close all logback resources (such as AsyncAppender worker threads).

@FilenkovMaxim
Copy link
Author

@tony19 thank you. I'll fix this.

@FilenkovMaxim
Copy link
Author

@tony19 I've checked. Without ci.autoConfig(); logback doesn't create new log file until app restart.
If I should not use ci.autoConfig(); how I can start writing logs to new file?

@tony19 tony19 closed this as completed in c7cb68c Dec 5, 2018
@tony19
Copy link
Owner

tony19 commented Dec 5, 2018

Fixed in v_1.3.0-3

@FilenkovMaxim
Copy link
Author

Hello.
I tried
implementation 'com.github.tony19:logback-android:1.3.0-3'
and

LoggerContext lc = (LoggerContext) LoggerFactory.getILoggerFactory();
    lc.reset();
    lc.stop();

but it still doesn't create new log file until app restart.

@tony19
Copy link
Owner

tony19 commented Feb 13, 2019

@FilenkovMaxim That's unrelated to this issue, which was about a resource leak. With your original code (including ci.autoConfig()), you'll no longer observe the resource-leak exceptions.

Btw, you should replace lc.reset() with lc.stop() (don't do both).

@lock
Copy link

lock bot commented Mar 15, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot added the archived label Mar 15, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants