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

#2168 Synchronizing extraction of pluggable transports on class to avoid... #2169

Merged
merged 1 commit into from Jan 13, 2015

Conversation

oxtoacart
Copy link
Contributor

... race condition in extracting file

@coveralls
Copy link

Coverage Status

Coverage increased (+0.26%) when pulling 154e95d on 2168 into f4079ac on issue-2164.

@myleshorton
Copy link
Contributor

I would consider using FileLock in LanternUtils instead, as it would solve the problem more generally and would isolate the locking to the file itself.

@oxtoacart
Copy link
Contributor Author

Neat idea. Do you mean org.littleshoot.util.FileLockUtils ?

@myleshorton
Copy link
Contributor

No java.nio.channels.FileLock directly -- something like

FileOutputStream out = new FileOutputStream(file);
try {
    java.nio.channels.FileLock lock = out.getChannel().lock();
    try {
        // move file in here.
    } finally {
        lock.release();
    }
} finally {
    in.close();
}

@oxtoacart
Copy link
Contributor Author

Gotcha. And this is to prevent problems when running two instances of Lantern, right?

@myleshorton
Copy link
Contributor

It's definitely a bit more verbose, but more robust I think too.

@myleshorton
Copy link
Contributor

Gotcha. And this is to prevent problems when running two instances of Lantern, right?

Yeah, I'm basically thinking we just lock the flashlight file, although now that I think about it that'll just lock to the process not the thread -- maybe ignore me.

@oxtoacart
Copy link
Contributor Author

although now that I think about it that'll just lock to the process not the thread

Correct. From the Javadoc for java.nio.channels.FileLock:

"File locks are held on behalf of the entire Java virtual machine. They are not suitable for controlling access to a file by multiple threads within the same virtual machine."

@myleshorton
Copy link
Contributor

OK, let's just go with your solution. I'm always wary of synchronizing on a class, but I can't think of anything better here.

myleshorton added a commit that referenced this pull request Jan 13, 2015
#2168 Synchronizing extraction of pluggable transports on class to avoid...
@myleshorton myleshorton merged commit 78a053a into issue-2164 Jan 13, 2015
@myleshorton myleshorton deleted the 2168 branch January 13, 2015 23:30
@oxtoacart
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants