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

rows.Close() must not overwrite buffer #903

Closed
methane opened this issue Dec 3, 2018 · 5 comments
Closed

rows.Close() must not overwrite buffer #903

methane opened this issue Dec 3, 2018 · 5 comments
Labels
bug high priority

Comments

@methane
Copy link
Member

methane commented Dec 3, 2018 •
edited
Loading

Read these:

From Go 1.10, it's not allowed anymore. Maybe, it should be avoided from Go 1.9.
We should use another buffer...

This issue caused #902.
@methane methane mentioned this issue Dec 3, 2018
Closed
@DoctorQ

This comment has been minimized.

Sign in to view
@methane

This comment has been minimized.

Sign in to view
@methane
Copy link
Member Author

methane commented Dec 3, 2018

I think avoiding one allocation for each query is too difficult.

methane added a commit to methane/mysql that referenced this issue Dec 3, 2018
@methane
Reset internal buffer when rows.Close() is called.
8d6f3b8 
Fixes go-sql-driver#903
@methane methane mentioned this issue Dec 3, 2018
Closed
5 tasks
methane added a commit to methane/mysql that referenced this issue Dec 3, 2018
@methane
Reset internal buffer when rows.Close() is called.
b0940b6 
Fixes go-sql-driver#903
@methane methane mentioned this issue Mar 8, 2019
Closed
@methane methane mentioned this issue Mar 29, 2019
Merged
5 tasks
@vmg vmg mentioned this issue Apr 1, 2019
Merged
5 tasks
@vmg
Copy link
Contributor

vmg commented Apr 1, 2019

I've implemented a potential fix in #943 and discussed several other approaches which I believe are inferior to the one I'm proposing. Feedback is welcome.

@methane methane changed the title rows.Close() must not overwrite buffer used by rows.Next() rows.Close() must not overwrite buffer Apr 2, 2019
@methane
Copy link
Member Author

methane commented Apr 2, 2019 •
edited
Loading

I created reproducer: https://gist.github.com/methane/418f872d788209116f8a5d08f756d838
go run -race reports race too.

To reproduce, we need to use sql.RawBytes or some Scanner object which doesn't deep copy []byte.

methane added a commit to methane/mysql that referenced this issue Apr 3, 2019
@methane
Reset internal buffer when rows.Close() is called.
55f4cae 
Fixes go-sql-driver#903
@methane methane mentioned this issue Apr 4, 2019
Closed
5 tasks
@methane methane closed this as completed in df597a2 Apr 4, 2019
@methane methane mentioned this issue Apr 4, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug high priority
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@vmg @methane @DoctorQ