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.

Sign up for GitHub

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

Jump to bottom

Fix null ref in persistence service #65646

Merged
merged 3 commits into from
Nov 28, 2022
Merged

Fix null ref in persistence service #65646

merged 3 commits into from
Nov 28, 2022

Conversation

CyrusNajmabadi
Copy link
Member

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner November 28, 2022 21:32
CyrusNajmabadi
CyrusNajmabadi commented Nov 28, 2022
View reviewed changes
src/Workspaces/Core/Portable/FindSymbols/Shared/AbstractSyntaxIndex_Persistence.cs
using var reader = ObjectReader.TryGetReader(stream, cancellationToken: cancellationToken);
if (reader != null)
return read(stringTable, reader, checksum);
if (stream != null)
Copy link
Member Author

Choose a reason for hiding this comment

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

stream can be null when no data is stored. didn't test the 'load first then save' scenario.

@CyrusNajmabadi
Copy link
Member Author

@ryzngard @dibarbet ptal.

@dibarbet
Copy link
Member

@CyrusNajmabadi what was the original null ref? the stream being null?

@CyrusNajmabadi
Copy link
Member Author

@CyrusNajmabadi what was the original null ref? the stream being null?

Yes.

ryzngard
ryzngard approved these changes Nov 28, 2022
View reviewed changes
src/Workspaces/Core/Portable/Storage/SQLite/v2/SQLitePersistentStorage.cs Outdated
@@ -45,6 +45,7 @@ internal sealed partial class SQLitePersistentStorage : AbstractPersistentStorag

private readonly string _insert_into_string_table_values_0 = $"insert into {StringInfoTableName}({DataColumnName}) values (?)";
private readonly string _select_star_from_string_table_where_0_limit_one = $"select * from {StringInfoTableName} where ({DataColumnName} = ?) limit 1";
private readonly string _select_star_from_string_table = $"select * from {StringInfoTableName}";
Copy link
Contributor

Choose a reason for hiding this comment

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

Not, remove as well

Copy link
Member Author

Choose a reason for hiding this comment

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

will do!

@CyrusNajmabadi CyrusNajmabadi mentioned this pull request Nov 28, 2022
Merged
@dibarbet
Copy link
Member

@CyrusNajmabadi what was the original null ref? the stream being null?

Yes.

hmm for some reason your original comment on it didn't show up when I first loaded the PR. shows up now though...

@CyrusNajmabadi CyrusNajmabadi mentioned this pull request Nov 28, 2022
Closed
@CyrusNajmabadi CyrusNajmabadi merged commit bebc5af into dotnet:main Nov 28, 2022
@ghost ghost added this to the Next milestone Nov 28, 2022
@CyrusNajmabadi CyrusNajmabadi deleted the fixNullRefPersistence branch November 28, 2022 23:50
allisonchou added a commit to allisonchou/roslyn that referenced this pull request Nov 29, 2022
@allisonchou
Revert "Merge pull request dotnet#65646 from CyrusNajmabadi/fixNullRe…
f86d611 
…fPersistence"

This reverts commit bebc5af, reversing
changes made to 1766515.
@allisonchou allisonchou modified the milestones: Next, 17.5 P2 Nov 30, 2022
@allisonchou allisonchou mentioned this pull request Dec 1, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryzngard ryzngard ryzngard approved these changes

@dibarbet dibarbet dibarbet approved these changes

Assignees
No one assigned
Labels
Area-IDE
Projects
None yet
Milestone
17.5 P2
Development

Successfully merging this pull request may close these issues.
4 participants
@CyrusNajmabadi @dibarbet @ryzngard @allisonchou