Skip to content

Add get_resource_id() function #5427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions Zend/tests/get_resource_id.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
--TEST--
get_resource_id() function
--FILE--
<?php

$file = fopen(__FILE__, 'r');

// get_resource_id() is equivalent to an integer cast.
var_dump(get_resource_id($file) === (int) $file);

// Also works with closed resources.
fclose($file);
var_dump(get_resource_id($file) === (int) $file);

?>
--EXPECT--
bool(true)
bool(true)
14 changes: 14 additions & 0 deletions Zend/zend_builtin_functions.c
Original file line number Diff line number Diff line change
Expand Up @@ -1505,6 +1505,20 @@ ZEND_FUNCTION(get_resource_type)
}
/* }}} */

/* {{{ proto int get_resource_id(resource res)
Get the resource ID for a given resource */
ZEND_FUNCTION(get_resource_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess resource-ids will be re-used across request boundaries? if so, should be mentioned somewhere

Copy link
Contributor

@TysonAndre TysonAndre Apr 21, 2020

Choose a reason for hiding this comment

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

I'd agree it should be documented. Obviously, this RFC doesn't change the existing behavior, changing it here is definitely out of scope, and the same note also applies to (int)$resource.
(e.g. "currently, resource ids are unique for the lifetime of execution (e.g. an http request in a web server), but this may change, because it's limited to 2**32 on 32-bit platforms)

I guess part of the issue is that it's using an associative array from integers to strings, and that zend_hash_index_del doesn't reset the last array pointer. I guess the only place where this could matter is that long-running 32-bit applications that create a lot of resources would possibly be affected by this (only able to create 4 billion ids in a lifetime for sockets/files, unless I misunderstood how this works), but for 64-bit applications, it doesn't matter.

Alternately, I guess it'd be possible to maintain a "free list" like what objects do so that resource ids can be reused.

php > for ($i = 0; $i < 3; $i++) { $fp = fopen("test$i.out", "w"); echo (int)$fp, "\n"; fclose($fp); $fp = null; }
2
3
4
php > $x = ['a', 'b'];
php > unset($x[1]);
php > $x[] = 'c';
php > var_export($x);
array (
  0 => 'a',
  2 => 'c',
)
ZEND_API void ZEND_FASTCALL zend_list_free(zend_resource *res)
{
	ZEND_ASSERT(GC_REFCOUNT(res) == 0);
	zend_hash_index_del(&EG(regular_list), res->handle);
}

Copy link
Member Author

Choose a reason for hiding this comment

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

{
zval *resource;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_RESOURCE(resource)
ZEND_PARSE_PARAMETERS_END();

RETURN_LONG(Z_RES_HANDLE_P(resource));
}
/* }}} */

/* {{{ proto array get_resources([string resouce_type])
Get an array with all active resources */
ZEND_FUNCTION(get_resources)
Expand Down
2 changes: 2 additions & 0 deletions Zend/zend_builtin_functions.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ function get_defined_vars(): array {}

function get_resource_type($res): string {}

function get_resource_id($res): int {}

function get_resources(string $type = UNKNOWN): array {}

function get_loaded_extensions(bool $zend_extensions = false): array {}
Expand Down
6 changes: 6 additions & 0 deletions Zend/zend_builtin_functions_arginfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_get_resource_type, 0, 1, IS_STRI
ZEND_ARG_INFO(0, res)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_get_resource_id, 0, 1, IS_LONG, 0)
ZEND_ARG_INFO(0, res)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_get_resources, 0, 0, IS_ARRAY, 0)
ZEND_ARG_TYPE_INFO(0, type, IS_STRING, 0)
ZEND_END_ARG_INFO()
Expand Down Expand Up @@ -244,6 +248,7 @@ ZEND_FUNCTION(get_declared_interfaces);
ZEND_FUNCTION(get_defined_functions);
ZEND_FUNCTION(get_defined_vars);
ZEND_FUNCTION(get_resource_type);
ZEND_FUNCTION(get_resource_id);
ZEND_FUNCTION(get_resources);
ZEND_FUNCTION(get_loaded_extensions);
ZEND_FUNCTION(get_defined_constants);
Expand Down Expand Up @@ -305,6 +310,7 @@ static const zend_function_entry ext_functions[] = {
ZEND_FE(get_defined_functions, arginfo_get_defined_functions)
ZEND_FE(get_defined_vars, arginfo_get_defined_vars)
ZEND_FE(get_resource_type, arginfo_get_resource_type)
ZEND_FE(get_resource_id, arginfo_get_resource_id)
ZEND_FE(get_resources, arginfo_get_resources)
ZEND_FE(get_loaded_extensions, arginfo_get_loaded_extensions)
ZEND_FE(get_defined_constants, arginfo_get_defined_constants)
Expand Down