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

Fix unclosed resource to prevent temp file deletion failure on Windows #24

Merged
merged 1 commit into from
Jul 16, 2019

Conversation

sakama
Copy link
Contributor

@sakama sakama commented Jul 16, 2019

This PR fixes a bug that temp file deletion fails on Windows.
This bug was reported by OSS guy.
https://translate.google.com/translate?hl=ja&sl=auto&tl=en&u=https%3A%2F%2Fteratail.com%2Fquestions%2F200148

org.embulk.exec.PartialExecutionException: org.embulk.config.ConfigException: Co
uldn't delete local file C:\Users\ADMINI~1\AppData\Local\Temp\2\embulk\2019-07-1
2 06-16-09.234 UTC\test000.00.csv3105253771120283850.tmp
        at org.embulk.exec.BulkLoader$LoaderState.buildPartialExecuteException(B
ulkLoader.java:340)
        at org.embulk.exec.BulkLoader.doRun(BulkLoader.java:566)
        at org.embulk.exec.BulkLoader.access$000(BulkLoader.java:35)
        at org.embulk.exec.BulkLoader$1.run(BulkLoader.java:353)
        at org.embulk.exec.BulkLoader$1.run(BulkLoader.java:350)
        at org.embulk.spi.Exec.doWith(Exec.java:22)
        at org.embulk.exec.BulkLoader.run(BulkLoader.java:350)
        at org.embulk.EmbulkEmbed.run(EmbulkEmbed.java:178)
        at org.embulk.EmbulkRunner.runInternal(EmbulkRunner.java:292)
        at org.embulk.EmbulkRunner.run(EmbulkRunner.java:156)
        at org.embulk.cli.EmbulkRun.runSubcommand(EmbulkRun.java:433)
        at org.embulk.cli.EmbulkRun.run(EmbulkRun.java:90)
        at org.embulk.cli.Main.main(Main.java:64)
Caused by: org.embulk.config.ConfigException: Couldn't delete local file C:\User
s\ADMINI~1\AppData\Local\Temp\2\embulk\2019-07-12 06-16-09.234 UTC\test000.00.cs
v3105253771120283850.tmp
        at org.embulk.output.ftp.FtpFileOutputPlugin$FtpFileOutput$1.call(FtpFil
eOutputPlugin.java:269)
        at org.embulk.output.ftp.FtpFileOutputPlugin$FtpFileOutput$1.call(FtpFil
eOutputPlugin.java:245)
        at org.embulk.spi.util.RetryExecutor.run(RetryExecutor.java:81)
        at org.embulk.spi.util.RetryExecutor.runInterruptible(RetryExecutor.java
:62)
        at org.embulk.output.ftp.FtpFileOutputPlugin$FtpFileOutput.uploadFile(Ft
pFileOutputPlugin.java:245)
        at org.embulk.output.ftp.FtpFileOutputPlugin$FtpFileOutput.finish(FtpFil
eOutputPlugin.java:233)
        at org.embulk.spi.util.FileOutputOutputStream.close(FileOutputOutputStre
am.java:94)
        at sun.nio.cs.StreamEncoder.implClose(StreamEncoder.java:320)
        at sun.nio.cs.StreamEncoder.close(StreamEncoder.java:149)
        at java.io.OutputStreamWriter.close(OutputStreamWriter.java:233)
        at java.io.BufferedWriter.close(BufferedWriter.java:266)
        at org.embulk.spi.util.LineEncoder.finish(LineEncoder.java:90)
        at org.embulk.standards.CsvFormatterPlugin$1.finish(CsvFormatterPlugin.j
ava:194)
        at org.embulk.spi.FileOutputRunner$DelegateTransactionalPageOutput.finis
h(FileOutputRunner.java:154)
        at org.embulk.spi.PageBuilder.finish(PageBuilder.java:228)
        at org.embulk.standards.CsvParserPlugin.run(CsvParserPlugin.java:377)
        at org.embulk.spi.FileInputRunner.run(FileInputRunner.java:140)
        at org.embulk.spi.util.Executors.process(Executors.java:62)
        at org.embulk.spi.util.Executors.process(Executors.java:38)
        at org.embulk.exec.LocalExecutorPlugin$DirectExecutor$1.call(LocalExecut
orPlugin.java:170)
        at org.embulk.exec.LocalExecutorPlugin$DirectExecutor$1.call(LocalExecut
orPlugin.java:167)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.
java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor
.java:624)
        at java.lang.Thread.run(Thread.java:748)
        Suppressed: org.embulk.config.ConfigException: Couldn't delete local fil
e C:\Users\ADMINI~1\AppData\Local\Temp\2\embulk\2019-07-12 06-16-09.234 UTC\test
001.00.csv6718041067910710146.tmp
                ... 25 more
        Suppressed: org.embulk.config.ConfigException: Couldn't delete local fil
e C:\Users\ADMINI~1\AppData\Local\Temp\2\embulk\2019-07-12 06-16-09.234 UTC\test
002.00.csv9129426862521090912.tmp
                ... 25 more

Error: org.embulk.config.ConfigException: Couldn't delete local file C:\Users\AD
MINI~1\AppData\Local\Temp\2\embulk\2019-07-12 06-16-09.234 UTC\test000.00.csv310
5253771120283850.tmp

The cause is probably same with embulk/embulk-output-azure_blob_storage#14
If resource isn't closed, Java program fails to delete temp file.

I don't have an Windows env. I'll ask certain guy to check after release.

Copy link

@tvhung83 tvhung83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tvhung83
Copy link

Oh wait, on a 2nd thought, do you think it's related to space in filename?

@sakama
Copy link
Contributor Author

sakama commented Jul 16, 2019

I first thought so. But maybe no.
Tthis plugin uses Exec.getTempFileSpace().createTempFile(String filePath, String suffix) to generate temp file which is provided by Embulk core.
https://github.com/embulk/embulk-output-ftp/pull/24/files#diff-6ccad54c987c2e6cfc57194f7d8dededR193

If this method has a bug, the problem should be reported also at other plugins that have a same logic.
Azure Blog Storage plugin as well.
https://github.com/embulk/embulk-output-azure_blob_storage/pull/14/files#diff-f979b3a87c261b4c14e5f8dac5e8d32cL165

@tvhung83
Copy link

Ah, I see, then it's still good!

@sakama sakama merged commit d035113 into master Jul 16, 2019
@sakama
Copy link
Contributor Author

sakama commented Jul 16, 2019

Thanks!

@sakama sakama deleted the close-resource branch July 16, 2019 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants