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

FileTokenCacheStore should clean up after failing to open the cache file #1440

Open
microsoftjoe opened this issue Jul 9, 2019 · 5 comments
Assignees
Labels
Bug - P2 A problem that needs to be fixed for a feature to function as intended Internal Issue created by a project contributor Issue Triage The engineering team has looked into the issue, understood the issue, labelled/classified the issue

Comments

@microsoftjoe
Copy link
Contributor

We've seen exception reports in our telemetry that indicate that FileTokenCacheStore is throwing an IllegalStateException on line 117.
Code inspection shows that the reason is that the cache file couldn't be deserialized into an instance of MemoryTokenCacheStore. The comments indicate that once this happens, the cache file will not work again.
If this is the case, the file should be deleted and an instance of MemoryTokenCacheStore created so that the FileTokenCacheStore object can be successfully instantiated and the cache file can be re-created on the next write.

@hamiltonha
Copy link
Contributor

what version of ADAL were you using and was it repro-ing on the latest version?

@microsoftjoe
Copy link
Contributor Author

I wasn't using any version of ADAL, these telemetry events come from partner teams who are using various versions.
However, the file in question was last updated with a formatting change in March, 2018, so the version shouldn't matter very much.
What's happening is that the attempt to read the file cache around line 91 throws an IOException, probably because of some file corruption. We know from this that the file exists but can't be successfully opened.
There is even a comment at line 114 describing that the FileTokenCacheStore is broken and will not work again, but it then just throws an exception rather than attempting to clean up.
Because the file isn't cleaned up, the exception happens every time the FileTokenCacheStore is instantiated and ADAL is essentially broken at this point.

@microsoftjoe
Copy link
Contributor Author

Also, we don't have any theory or data about why the file was corrupted or the nature of that corruption. All we know is that the exception was thrown when the file existed. My point here is that it would make sense to clean up this file if you know that it will never successfully be opened again.

@shoatman
Copy link
Contributor

shoatman commented Nov 5, 2019

Thanks Joe. Do you happen to know which app or apps these are? I'm curious since the FileTokenCacheStore is not the default and I'd like to understand why they chose to use the FileTokenCacheStore as well as get a sense of how many people are impacted by this. Clearly when the impact is experience the only remediation is likely uninstall/re-install....

@microsoftjoe
Copy link
Contributor Author

Sorry, this was a while ago, and our telemetry data isn't maintained for more than 30 days (usually less due to storage constraints). We stopped putting ADAL errors in our telemetry a few months ago, since they're generally not germane to MAM failures, and to help with the aforementioned storage constraints ;)

@hamiltonha hamiltonha added the bug label Nov 13, 2019
@hamiltonha hamiltonha assigned hamiltonha and shoatman and unassigned hamiltonha Nov 13, 2019
@iambmelt iambmelt added Bug - P2 A problem that needs to be fixed for a feature to function as intended Internal Issue created by a project contributor Issue Triage The engineering team has looked into the issue, understood the issue, labelled/classified the issue and removed bug labels Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug - P2 A problem that needs to be fixed for a feature to function as intended Internal Issue created by a project contributor Issue Triage The engineering team has looked into the issue, understood the issue, labelled/classified the issue
Projects
None yet
Development

No branches or pull requests

4 participants