Re: BUG #16329: Valgrind detects an invalid read when building a gistindex with buffering
От | Alexander Lakhin |
---|---|
Тема | Re: BUG #16329: Valgrind detects an invalid read when building a gistindex with buffering |
Дата | |
Msg-id | 93ca4540-0016-8217-2b31-535613499987@gmail.com обсуждение исходный текст |
Ответ на | Re: BUG #16329: Valgrind detects an invalid read when building a gist index with buffering (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: BUG #16329: Valgrind detects an invalid read when building a gistindex with buffering
|
Список | pgsql-bugs |
Hello Tom,
05.05.2020 21:59, Tom Lane wrote:
05.05.2020 21:59, Tom Lane wrote:
Thanks for your review!Could you please let me know if the fix is incorrect (or not elaborated enough to be included in the upcoming releases) or this issue is false positive?I took a look at this. I see the hazard, but I'm not sure that I like your proposed quick-fix. Essentially, the problem is that gistBuildCallback is supposing that the tuple it creates will survive the execution of its subroutines, which in fact it won't always. But that means that at some point the tuple pointer it's passed down to those subroutines becomes a dangling pointer. What guarantee would we have that the subroutines don't access the tuple after that point?
Yes, I'm not excited by this fix too, so I'll try to find another not so quick way to fix it.
Please look at the improved test that makes the code coverage for gistbuildbuffers.c almost 100%.Or, if we do just hack it as you suggest, there had better be a couple of fat comments in gistBufferingBuildInsert warning about the hazards. I was somewhat dismayed to realize from looking at the code coverage report that we have exactly zero test coverage of the gist buffering build code paths today. That's just awful; how did the feature get committed with no test coverage? Your proposed test additions raise the coverage in gistbuild.c and gistbuildbuffers.c to something respectable, at least. But it looks like there are still some potentially-delicate paths that aren't tested, notably the "have to stop buffering" business. Can we do better at reasonable cost, or are those paths just never reached without huge data volume? (Could we make them more reachable by turning down maintenance_work_mem or some other setting?)
Best regards,
Alexander
Вложения
В списке pgsql-bugs по дате отправления: