Bug #74011 [Asn->Csd]: Access Violation on ZTS Embed SAPI Shutdown

[email protected]
Newsgroups php.bugs
Message-ID <[email protected]>
Edit report at https://bugs.php.net/bug.php?id=74011&edit=1

 ID:                 74011
 Updated by:         [email protected]
 Reported by:        maroszek at gmx dot net
 Summary:            Access Violation on ZTS Embed SAPI Shutdown
-Status:             Assigned
+Status:             Closed
 Type:               Bug
 Package:            Reproducible crash
 Operating System:   Windows 10
 PHP Version:        7.1.1
 Assigned To:        ab
 Block user comment: N
 Private report:     N

 New Comment:

@maroszek many thanks for the checks. I've compiled the snippet from the description and your latest one based on pure sapi/embed. With the embed snippet it seems hard to reproduce the crash even with 7.0, however with the original snippet i can confirm there are no crashes with master anymore. It was expected, however. Thus, closing this one and the predecessor ticket. Well, as an issue is an issue, that's fine with a custom SAPI, just it was a bit misleading to have it linked to embed where it was a custom SAPI indeed. Looking forward to 7.2 anyway, with the huge stability increase in this regard :)

Please, lets investigate the issue with tsrm_startup you've mentioned in the separate ticket, would need some backtrace and the usual stuff. Please file a ticket, even you could assign it to me right away.

Thanks.


Previous Comments:
------------------------------------------------------------------------
[2017-03-10 08:53:37] maroszek at gmx dot net

Thanks for fixing the issue! For Linux it works as is - for Windows i had to make a small change to my custom SAPI.

I was using this line, which was causing crashes on Windows.

    tsrm_startup(128, 1, 0, NULL);

After changing it to the way the embed library does it the crashes went away.

    tsrm_startup(1, 1, 0, NULL);

What i don't understand, is how a parameter that is named expected_threads can causes crashes, after setting it to a sane value.

Here is my reworked crash example which uses the embed library: https://gist.github.com/paresy/3cbd4c6a469511ac7479aa0e7c42fea7
I will try my best to post next reports based on the official embed implementation! :)

------------------------------------------------------------------------
[2017-03-08 17:49:31] [email protected]

@maroszek, could you please check the current master. The interned strings are now supported in thread safe builds, so the issues from #71115 sohuld be fixed as well.

Thanks.

------------------------------------------------------------------------
[2017-02-22 01:11:23] [email protected]

@maroszek, there is a work in progress branch dealing with interned strings https://github.com/php/php-src/pull/2390 , which are the cause of issues in TS builds currently. The patch linked in the description was doing just a local fix in this direction, the PR targets a robust implementation.

Regarding the code you've linked - I still couldn't come to it, but was checking the code itself. Also checked and read the old linked issue. The thing is, and that's what wondered me earlier as well - it doesn't look like you indeed use the embed SAPI, the provided init/shutdown routines are not used. It looks like you develop some more or less new SAPI, based on embed. And in this case, it is a completely different story, which should not be sold as a bug in the embed SAPI itself. I also read other users comments in the old ticket about this.

With the core SAPIs - yes, the bugs are targeted as bug reports. But with a custom approach, that reuses things partially - it is hard to say where it's a mistake in your own code or a real bug in the core. If you're curious, you might check the linked PR. Given also that we have TS issues indeed, maybe it'd make sense to keep this bug. But otherwise - i barely see such code as a core bug in principle. Usually you would first ask for help on mailing lists about a custom SAPI development. I'd frankly see more willingly and would find more simple a reproducer with an exact embed SAPI, without ary unnecessary customization.

Thanks.

------------------------------------------------------------------------
[2017-02-20 11:18:58] maroszek at gmx dot net

@ab: Sorry for being so persistent. Do you have any updates? Is there anything i can do to help? Is there any summary of related issues and ideas for fixes i could potentially work on?

Thanks!

------------------------------------------------------------------------
[2017-02-09 12:31:51] [email protected]

@maroszek, i'm investigating on the similar issues right now, which seem to belong to the same bundle. So far i don't see any quick solution to this :( But the work continues.

Thanks.

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=74011


--
Edit this bug report at https://bugs.php.net/bug.php?id=74011&edit=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.