GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [3.7] gh-80254: Disallow recursive usage of cursors in sqlite3 converters by erlend-aasland · Pull Request #92334 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented May 5, 2022

Copy link
Copy Markdown
Contributor

(cherry picked from commit c908dc5)

Co-authored-by: Sergey Fedoseev [email protected]
Co-authored-by: Jelle Zijlstra [email protected]

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
erlend-aasland and others added 3 commits May 4, 2022 23:50
@erlend-aasland @sir-sigurd @JelleZijlstra
[3.7] pythongh-80254: Disallow recursive usage of cursors in `sqlite3…
19f2c47 
…` converters

(cherry picked from commit c908dc5)

Co-authored-by: Sergey Fedoseev <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
@erlend-aasland
Fix ref leak in pysqlite_cursor_iternext
f6c89bc
@erlend-aasland
Explicitly free resources at test tearDown()
d52fd31
@bedevere-bot bedevere-bot added the awaiting review label May 5, 2022
@erlend-aasland erlend-aasland added 🔨 test-with-buildbots Test PR w/ buildbots; report in status section and removed awaiting review labels May 5, 2022
@bedevere-bot

bedevere-bot commented May 5, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit d52fd31 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 5, 2022
@erlend-aasland
erlend-aasland requested review from JelleZijlstra and ambv May 5, 2022 12:13
@erlend-aasland

erlend-aasland commented May 5, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

CI cannot find merge base, and decides that this is not worthy running the tests. Buildbots seem to ignore this fact though, so I guess that it's fine if they are green.

UPDATE: a little bit too fast there; AMD64 FreeBSD Non-Debug is green, but it did not run. It failed before even compiling.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
This was referenced May 5, 2022
gh-80254: Disallow recursive usage of cursors in sqlite3 converters #29054
Merged
sqlite crashes with converters mutating cursor #80254
Closed
@erlend-aasland

erlend-aasland commented May 5, 2022

Copy link
Copy Markdown
Contributor Author

This buildbot run does not look good. Do you experience this often on the 3.7 branch, @ambv?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
JelleZijlstra
JelleZijlstra approved these changes May 5, 2022
View reviewed changes

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We're not allowed to merge until regular CI passes (the check is required). Code looks good to me but we'll have to figure out what's up with CI.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added the awaiting merge label May 5, 2022
@erlend-aasland

erlend-aasland commented May 5, 2022

Copy link
Copy Markdown
Contributor Author

I think it is related to 4ac923f
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

erlend-aasland commented May 5, 2022

Copy link
Copy Markdown
Contributor Author

See also GH-21806
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

erlend-aasland commented May 5, 2022

Copy link
Copy Markdown
Contributor Author

I think backporting both GH-24004 and GH-21806 will fix it. Maybe not both are required, but I think both should be backported.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JelleZijlstra

JelleZijlstra commented May 5, 2022

Copy link
Copy Markdown
Member

I think backporting both GH-24004 and GH-21806 will fix it. Maybe not both are required, but I think both should be backported.

Seems reasonable. I triggered #92341 and #92342, but I am not authorized to merge to the 3.7 branch.
erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JelleZijlstra JelleZijlstra mentioned this pull request May 5, 2022
[3.7] bpo-40548: Fix "Check for source changes (pull_request)" GH Action job (GH-21806) #92342
Merged
@erlend-aasland

erlend-aasland commented May 5, 2022

Copy link
Copy Markdown
Contributor Author

Great! I'll pull in 3.7 when the backports land.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented May 5, 2022

Copy link
Copy Markdown
Member

The 3.7 branch no longer accept bugfixes, only security fixes: https://devguide.python.org/#status-of-python-branches

Do you consider that this change fix a security issue?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ned-deily

ned-deily commented May 6, 2022

Copy link
Copy Markdown
Member

Do you consider that this change fix a security issue?

Yes, see discussion here.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ned-deily
Merge branch '3.7' into backport-sqlite-converter-segfault-3.7
fe0d218
@ned-deily
ned-deily merged commit 2a353b2 into python:3.7 May 23, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label May 23, 2022
@erlend-aasland
erlend-aasland deleted the backport-sqlite-converter-segfault-3.7 branch May 23, 2022 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes
@ambv ambv Awaiting requested review from ambv

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@erlend-aasland @bedevere-bot @JelleZijlstra @vstinner @ned-deily
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.