search

edgewall / genshi

Python toolkit for generation of output for the web
http://genshi.edgewall.org
Other
86 stars 35 forks source link

Fix reference leak in Markup.join C implementation #47

Closed hodgestar closed 3 years ago

hodgestar commented 3 years ago

When support for iterator arguments was added to the Markup.join implementation in 99ad51d the temporary tuple was replaced by a temporary list and a switch was made from PyTuple_SET_ITEM (which steals a reference) to PyList_Append (which does not) without adding the necessary Py_DECREF for the added item.

This PR adds the necessary reference.

Thanks you @eug3nix for reporting the issue in #46 and helping debug it. Would you mind checking that this PR solves your original issue?

eug3nix commented 3 years ago

I have installed the updated version and can confirm that the issue is resolved. Thank you @hodgestar !

hodgestar commented 3 years ago

Bleh. I forgot Travis went away so I should move the CI to GitHub Actions before merging this. I'll try do that over the weekend.

rjollos commented 3 years ago

Bleh. I forgot Travis went away so I should move the CI to GitHub Actions before merging this. I'll try do that over the weekend.

It was migrated here: https://app.travis-ci.com/github/edgewall/genshi

But appears the last 4 commits to master haven't triggered a build.

hodgestar commented 3 years ago

But appears the last 4 commits to master haven't triggered a build.

Exactly. :/

FelixSchwarz commented 3 years ago

In Travis I see something like "Could not authorize build request for edgewall/genshi." Also other Travis did run for other github projects of mine a few weeks ago so I think this more of a genshi setup issue. Maybe try to reauthorize Travis?

hodgestar commented 3 years ago

In Travis I see something like "Could not authorize build request for edgewall/genshi."

Thanks @FelixSchwarz. That led me to https://stackoverflow.com/questions/41034694/travis-could-not-authorize-build-request/41078031 and indeed no billing plan had been selected for the Edgewall organization. I selected the Free plan. Hopefully that was the right thing to do. :)

hodgestar commented 3 years ago

All the builds have passed in https://app.travis-ci.com/github/edgewall/genshi/jobs/535510039 which I triggered manually, so I'm going to merge this. The PR itself doesn't seem to have been updated with the results, but maybe that will happen automatically on commits again in future.