Re: TestBot job 89319 results: [PATCH 1/4] msxml3/tests: Add more tests for processing instruction attributes.

[Dmitry Timoshkov]

Marvin  wrote:
> VM                   Status   Failures Command
> build                success           
> w2008s64             success  0        msxml3_test.exe domdoc
> w7u_2qxl             success  0        msxml3_test.exe domdoc
> w7u_adm              success  0        msxml3_test.exe domdoc
> w7u_el               success  0        msxml3_test.exe domdoc
> w8                   success  0        msxml3_test.exe domdoc
> w8adm                success  0        msxml3_test.exe domdoc
> w864                 success  0        msxml3_test.exe domdoc
> w1064v1507           success  0        msxml3_test.exe domdoc
> w1064v1809           success  0        msxml3_test.exe domdoc
> w1064                success  0        msxml3_test.exe domdoc
> w10pro64             success  0        msxml3_test.exe domdoc
> wvistau64            failed   1        msxml3_test64.exe domdoc
> w2008s64             failed   1        msxml3_test64.exe domdoc
> w864                 failed   1        msxml3_test64.exe domdoc
> w1064v1507           failed   1        msxml3_test64.exe domdoc
> w1064v1809           failed   1        msxml3_test64.exe domdoc
> w1064                failed   1        msxml3_test64.exe domdoc
> w1064_2qxl           failed   1        msxml3_test64.exe domdoc
> w10pro64             failed   1        msxml3_test64.exe domdoc
> w10pro64_ar          failed   1        msxml3_test64.exe domdoc
> w10pro64_he          failed   1        msxml3_test64.exe domdoc
> w10pro64_ja          failed   1        msxml3_test64.exe domdoc
> w10pro64_zh_CN       failed   1        msxml3_test64.exe domdoc
> debiant2             success  0        
> debiant2             failed   1
>
> You can also see the results at:
> https://testbot.winehq.org/JobDetails.pl?Key=89319

Same patch sent separately runs without failures:
https://testbot.winehq.org/JobDetails.pl?Key=89342

TestBot job 89319 results: [PATCH 1/4] msxml3/tests: Add more tests for processing instruction attributes.

[Zebediah Figura (she/her)]

On 4/26/21 7:53 AM, Dmitry Timoshkov wrote:
> Marvin  wrote:
> > VM                   Status   Failures Command
> > build                success
> > w2008s64             success  0        msxml3_test.exe domdoc
> > w7u_2qxl             success  0        msxml3_test.exe domdoc
> > w7u_adm              success  0        msxml3_test.exe domdoc
> > w7u_el               success  0        msxml3_test.exe domdoc
> > w8                   success  0        msxml3_test.exe domdoc
> > w8adm                success  0        msxml3_test.exe domdoc
> > w864                 success  0        msxml3_test.exe domdoc
> > w1064v1507           success  0        msxml3_test.exe domdoc
> > w1064v1809           success  0        msxml3_test.exe domdoc
> > w1064                success  0        msxml3_test.exe domdoc
> > w10pro64             success  0        msxml3_test.exe domdoc
> > wvistau64            failed   1        msxml3_test64.exe domdoc
> > w2008s64             failed   1        msxml3_test64.exe domdoc
> > w864                 failed   1        msxml3_test64.exe domdoc
> > w1064v1507           failed   1        msxml3_test64.exe domdoc
> > w1064v1809           failed   1        msxml3_test64.exe domdoc
> > w1064                failed   1        msxml3_test64.exe domdoc
> > w1064_2qxl           failed   1        msxml3_test64.exe domdoc
> > w10pro64             failed   1        msxml3_test64.exe domdoc
> > w10pro64_ar          failed   1        msxml3_test64.exe domdoc
> > w10pro64_he          failed   1        msxml3_test64.exe domdoc
> > w10pro64_ja          failed   1        msxml3_test64.exe domdoc
> > w10pro64_zh_CN       failed   1        msxml3_test64.exe domdoc
> > debiant2             success  0
> > debiant2             failed   1
> >
> > You can also see the results at:
> > https://testbot.winehq.org/JobDetails.pl?Key=89319
>
> Same patch sent separately runs without failures:
> https://testbot.winehq.org/JobDetails.pl?Key=89342

The tests crash with STATUS_HEAP_CORRUPTION, which strikes me as 
something that could cause inconsistent behaviour. Are you sure that 
you're not introducing a failure here?

TestBot job 89319 results: [PATCH 1/4] msxml3/tests: Add more tests for processing instruction attributes.

[Dmitry Timoshkov]

"Zebediah Figura (she/her)"  wrote:
> > > You can also see the results at:
> > > https://testbot.winehq.org/JobDetails.pl?Key=89319
> >
> > Same patch sent separately runs without failures:
> > https://testbot.winehq.org/JobDetails.pl?Key=89342
>
> The tests crash with STATUS_HEAP_CORRUPTION, which strikes me as 
> something that could cause inconsistent behaviour. Are you sure that 
> you're not introducing a failure here?

Thanks for the hint. Running with warn+heap helped to find the reason
of the heap corruption, it's 'p[GlobalSize(global)] = 0;' statements.
This revealed another bug in msxml3 - it doesn't NUL terminate the stream on
::save(). After fixing that and removing 'p[] = 0' the processing instruction
tests stopped to crash.

P.S.
warn+heap still makes msxml3 domdoc tests crash, however that looks like
another bug somewhere.

TestBot job 89319 results: [PATCH 1/4] msxml3/tests: Add more tests for processing instruction attributes.

[Dmitry Timoshkov]

Dmitry Timoshkov  wrote:
> "Zebediah Figura (she/her)"  wrote:
> > > > You can also see the results at:
> > > > https://testbot.winehq.org/JobDetails.pl?Key=89319
> > >
> > > Same patch sent separately runs without failures:
> > > https://testbot.winehq.org/JobDetails.pl?Key=89342
> >
> > The tests crash with STATUS_HEAP_CORRUPTION, which strikes me as 
> > something that could cause inconsistent behaviour. Are you sure that 
> > you're not introducing a failure here?
>
> Thanks for the hint. Running with warn+heap helped to find the reason
> of the heap corruption, it's 'p[GlobalSize(global)] = 0;' statements.
> This revealed another bug in msxml3 - it doesn't NUL terminate the stream on
> ::save(). After fixing that and removing 'p[] = 0' the processing instruction
> tests stopped to crash.
>
> P.S.
> warn+heap still makes msxml3 domdoc tests crash, however that looks like
> another bug somewhere.

That another crash happens at the end of test_get_ownerDocument() because
priv->properties of doc1 and doc_owner have the same pointer value, and
releasing doc_owner after doc1 leads to a double free and heap corruption.

Nikolay, could you please have a look?

TestBot job 89319 results: [PATCH 1/4] msxml3/tests: Add more tests for processing instruction attributes.

[Nikolay Sivov]

On Tue, 27 Apr 2021 at 15:38, Dmitry Timoshkov  wrote:
> Dmitry Timoshkov  wrote:
> > "Zebediah Figura (she/her)"  wrote:
> > > > > You can also see the results at:
> > > > > https://testbot.winehq.org/JobDetails.pl?Key=89319
> > > >
> > > > Same patch sent separately runs without failures:
> > > > https://testbot.winehq.org/JobDetails.pl?Key=89342
> > >
> > > The tests crash with STATUS_HEAP_CORRUPTION, which strikes me as
> > > something that could cause inconsistent behaviour. Are you sure that
> > > you're not introducing a failure here?
> >
> > Thanks for the hint. Running with warn+heap helped to find the reason
> > of the heap corruption, it's 'p[GlobalSize(global)] = 0;' statements.
> > This revealed another bug in msxml3 - it doesn't NUL terminate the
>
> stream on
> > ::save(). After fixing that and removing 'p[] = 0' the processing
>
> instruction
> > tests stopped to crash.
> >
> > P.S.
> > warn+heap still makes msxml3 domdoc tests crash, however that looks like
> > another bug somewhere.
>
> That another crash happens at the end of test_get_ownerDocument() because
> priv->properties of doc1 and doc_owner have the same pointer value, and
> releasing doc_owner after doc1 leads to a double free and heap corruption.
>
> Nikolay, could you please have a look?

I won’t have time for that in coming days.
> --
> Dmitry.

TestBot job 89319 results: [PATCH 1/4] msxml3/tests: Add more tests for processing instruction attributes.

[Zebediah Figura (she/her)]

On 4/27/21 7:37 AM, Dmitry Timoshkov wrote:
> Dmitry Timoshkov  wrote:
> > "Zebediah Figura (she/her)"  wrote:
> > > > > You can also see the results at:
> > > > > https://testbot.winehq.org/JobDetails.pl?Key=89319
> > > >
> > > > Same patch sent separately runs without failures:
> > > > https://testbot.winehq.org/JobDetails.pl?Key=89342
> > >
> > > The tests crash with STATUS_HEAP_CORRUPTION, which strikes me as
> > > something that could cause inconsistent behaviour. Are you sure that
> > > you're not introducing a failure here?
> >
> > Thanks for the hint. Running with warn+heap helped to find the reason
> > of the heap corruption, it's 'p[GlobalSize(global)] = 0;' statements.
> > This revealed another bug in msxml3 - it doesn't NUL terminate the stream on
> > ::save(). After fixing that and removing 'p[] = 0' the processing instruction
> > tests stopped to crash.
> >
> > P.S.
> > warn+heap still makes msxml3 domdoc tests crash, however that looks like
> > another bug somewhere.
>
> That another crash happens at the end of test_get_ownerDocument() because
> priv->properties of doc1 and doc_owner have the same pointer value, and
> releasing doc_owner after doc1 leads to a double free and heap corruption.
>
> Nikolay, could you please have a look?

That's a rather old bug; unfortunately it's been independently 
discovered several times by now...

TestBot job 89319 results: [PATCH 1/4] msxml3/tests: Add more tests for processing instruction attributes.

[Francois Gouget]

On Tue, 27 Apr 2021, Dmitry Timoshkov wrote:
[...]
> That another crash happens at the end of test_get_ownerDocument() because
> priv->properties of doc1 and doc_owner have the same pointer value, and
> releasing doc_owner after doc1 leads to a double free and heap corruption.
>
> Nikolay, could you please have a look?

I'm one of the rediscoverers and I (re)sent a patch:
https://www.winehq.org/pipermail/wine-devel/2021-May/186299.html

TestBot job 89319 results: [PATCH 1/4] msxml3/tests: Add more tests for processing instruction attributes.

[Dmitry Timoshkov]

Francois Gouget  wrote:
> On Tue, 27 Apr 2021, Dmitry Timoshkov wrote:
> [...]
> > That another crash happens at the end of test_get_ownerDocument() because
> > priv->properties of doc1 and doc_owner have the same pointer value, and
> > releasing doc_owner after doc1 leads to a double free and heap corruption.
> >
> > Nikolay, could you please have a look?
>
> I'm one of the rediscoverers and I (re)sent a patch:
> https://www.winehq.org/pipermail/wine-devel/2021-May/186299.html

Thanks for working on this! Still, I'm waiting for a review of this patchset
from the maintainer. Nikolay, could you please either start reviewing assigned
to you patches in a reasonable time frame, or stop pretending to be a maintainer?

TestBot job 89319 results: [PATCH 1/4] msxml3/tests: Add more tests for processing instruction attributes.

[Nikolay Sivov]

On 5/6/21 5:41 PM, Dmitry Timoshkov wrote:
> Francois Gouget  wrote:
> > On Tue, 27 Apr 2021, Dmitry Timoshkov wrote:
> > [...]
> > > That another crash happens at the end of test_get_ownerDocument() because
> > > priv->properties of doc1 and doc_owner have the same pointer value, and
> > > releasing doc_owner after doc1 leads to a double free and heap corruption.
> > >
> > > Nikolay, could you please have a look?
> >
> > I'm one of the rediscoverers and I (re)sent a patch:
> > https://www.winehq.org/pipermail/wine-devel/2021-May/186299.html
>
> Thanks for working on this! Still, I'm waiting for a review of this patchset
> from the maintainer. Nikolay, could you please either start reviewing assigned
> to you patches in a reasonable time frame, or stop pretending to be a maintainer?

First you asked someone else to fix an older problem, that sort of
blocks test results from your own patches, instead of fixing it yourself.
Now you're complaining that it didn't happen fast enough?

TestBot job 89319 results: [PATCH 1/4] msxml3/tests: Add more tests for processing instruction attributes.

[Dmitry Timoshkov]

Nikolay Sivov  wrote:
> On 5/6/21 5:41 PM, Dmitry Timoshkov wrote:
> > Francois Gouget  wrote:
> > > On Tue, 27 Apr 2021, Dmitry Timoshkov wrote:
> > > [...]
> > > > That another crash happens at the end of test_get_ownerDocument() because
> > > > priv->properties of doc1 and doc_owner have the same pointer value, and
> > > > releasing doc_owner after doc1 leads to a double free and heap corruption.
> > > >
> > > > Nikolay, could you please have a look?
> > >
> > > I'm one of the rediscoverers and I (re)sent a patch:
> > > https://www.winehq.org/pipermail/wine-devel/2021-May/186299.html
> >
> > Thanks for working on this! Still, I'm waiting for a review of this patchset
> > from the maintainer. Nikolay, could you please either start reviewing assigned
> > to you patches in a reasonable time frame, or stop pretending to be a maintainer?
>
> First you asked someone else to fix an older problem

No, I didn't ask someone else (who is that by the way?), I went ahead and debuugged
the problem and sent an analysis to the list. Zebediah then replied:
"That's a rather old bug; unfortunately it's been independently 
discovered several times by now..."
> , that sort of
> blocks test results from your own patches, instead of fixing it yourself.

Do you mean that anyone sending patches must fix test failures caused
intermittently by not related things in *existing* tests?

And you while being a manintainer were ignoring this old bug for years
https://bugs.winehq.org/show_bug.cgi?id=43377
https://www.winehq.org/pipermail/wine-devel/2018-February/123378.html
https://www.winehq.org/pipermail/wine-devel/2018-February/123379.html
and now you're demanding me to fix that bug for you? That's rude and
incompentent on your part, to say the least.
> Now you're complaining that it didn't happen fast enough?

Are you really a maintainer, or just pretending to be the one?