search

fabianonline / telegram_backup

Java app to download all your telegram data.
GNU General Public License v3.0
447 stars 91 forks source link

Fixed issue #94 #99

Open Vest opened 6 years ago

Vest commented 6 years ago

Fixed issue #94 by closing open statements added out/ (IntelliJ) to .gitignore

Vest commented 6 years ago

I think it is possible to improve the fix by using try-with-resources for Kotlin. Unfortunately, the supported JDK should be increased from 1.6 (default for Kotlin) to 1.7 (1.8). E.g.

    compile "org.jetbrains.kotlin:kotlin-stdlib"
    compile "org.jetbrains.kotlin:kotlin-stdlib-jre7"

Can/should I try this option?

fabianonline commented 6 years ago

Thanks for this.

Unfortunately(-ish), I had also fixed the bug (and then forgot to push the code). I think closing the Statement object isn't the best way to proceed: There's just one of those and it's reused all over the place. So closing it could lead to funny problems way down the line.

Since the problem originates from ResultSets being open (which are implicitly closed by calling Statement.close()), my code looks at closing them explicitly after they're used.

As for bumping up the kotlin-stdlib: I'll have a look into that.

Vest commented 6 years ago

Ok, it is up to you to decide what to do with the PR. I have found that in some "helper" functions, such as queryInt, you get ResultSet from the connection and sometimes do not close it. Regarding the statements, I wanted to close the Statements in the Update method, because they were opened in the constructor (init), but I don't see that they were reused. So that is why I am quite safe with my approach.

Stdlib has "use" extension, it might help you to improve the mess with statements & result sets.

If you have reviewed my PR and don't need any change, feel free to close it.

leijurv commented 6 years ago

@fabianonline please merge =)