Convert shmop resources to opaque objects

We make `shmop_close()` a NOP, and deprecate the function right away;
detaching from SHM now happens when the wrapper object is freed.

8 files changed, 112 insertions, 105 deletions

diff --git a/NEWS b/NEWS
index 3732e9946d..be5aae762a 100644
--- a/NEWS
+++ b/NEWS
@@ -151,6 +151,9 @@ PHP                                                                        NEWS
     handlers). (bshaffer)
   . Fixed bug #73529 (session_decode() silently fails on wrong input). (cmb)
 
+- Shmop:
+  . Converted shmop resources to objects. (cmb)
+
 - SimpleXML:
   . Fixed bug #75245 (Don't set content of elements with only whitespaces).
     (eriklundin)
diff --git a/UPGRADING b/UPGRADING
index 165c133b70..f2210ee055 100644
--- a/UPGRADING
+++ b/UPGRADING
@@ -793,6 +793,13 @@ PHP 8.0 UPGRADE NOTES
 - PGSQL / PDO PGSQL:
   . The PGSQL and PDO PGSQL extensions now require at least libpq 9.1.
 
+- Shmop:
+  . shmop_open() will now return a Shmop object rather than a resource. Return
+    value checks using is_resource() should be replaced with checks for `false`.
+    The shmop_close() function no longer has an effect, and is deprecated;
+    instead the Shmop instance is automatically destroyed if it is no longer
+    referenced.
+
 ========================================
 10. New Global Constants
 ========================================
diff --git a/ext/shmop/php_shmop.h b/ext/shmop/php_shmop.h
index 659941aa3e..375c53eba8 100644
--- a/ext/shmop/php_shmop.h
+++ b/ext/shmop/php_shmop.h
@@ -32,26 +32,6 @@ PHP_MINFO_FUNCTION(shmop);
 # include "win32/ipc.h"
 #endif
 
-struct php_shmop
-{
-	int shmid;
-	key_t key;
-	int shmflg;
-	int shmatflg;
-	char *addr;
-	zend_long size;
-};
-
-typedef struct {
-	int le_shmop;
-} php_shmop_globals;
-
-#ifdef ZTS
-#define SHMOPG(v) TSRMG(shmop_globals_id, php_shmop_globals *, v)
-#else
-#define SHMOPG(v) (shmop_globals.v)
-#endif
-
 #else
 
 #define phpext_shmop_ptr NULL
diff --git a/ext/shmop/shmop.c b/ext/shmop/shmop.c
index 6f464e8745..c1e815eeb2 100644
--- a/ext/shmop/shmop.c
+++ b/ext/shmop/shmop.c
@@ -23,6 +23,7 @@
 
 #include "php.h"
 #include "php_ini.h"
+#include "Zend/zend_interfaces.h"
 #include "php_shmop.h"
 #include "shmop_arginfo.h"
 
@@ -38,14 +39,6 @@
 
 #include "ext/standard/info.h"
 
-#ifdef ZTS
-int shmop_globals_id;
-#else
-php_shmop_globals shmop_globals;
-#endif
-
-int shm_type;
-
 /* {{{ shmop_module_entry
  */
 zend_module_entry shmop_module_entry = {
@@ -66,22 +59,70 @@ zend_module_entry shmop_module_entry = {
 ZEND_GET_MODULE(shmop)
 #endif
 
-/* {{{ rsclean
- */
-static void rsclean(zend_resource *rsrc)
+typedef struct php_shmop
+{
+	int shmid;
+	key_t key;
+	int shmflg;
+	int shmatflg;
+	char *addr;
+	zend_long size;
+  zend_object std;
+} php_shmop;
+
+zend_class_entry *shmop_ce;
+static zend_object_handlers shmop_object_handlers;
+
+static inline php_shmop *shmop_from_obj(zend_object *obj)
+{
+	return (php_shmop *)((char *)(obj) - XtOffsetOf(php_shmop, std));
+}
+
+#define Z_SHMOP_P(zv) shmop_from_obj(Z_OBJ_P(zv))
+
+static zend_object *shmop_create_object(zend_class_entry *class_type)
+{
+	php_shmop *intern = zend_object_alloc(sizeof(php_shmop), class_type);
+
+	zend_object_std_init(&intern->std, class_type);
+	object_properties_init(&intern->std, class_type);
+	intern->std.handlers = &shmop_object_handlers;
+
+	return &intern->std;
+}
+
+static zend_function *shmop_get_constructor(zend_object *object)
+{
+	zend_throw_error(NULL, "Cannot directly construct Shmop, use shmop_open() instead");
+	return NULL;
+}
+
+static void shmop_free_obj(zend_object *object)
 {
-	struct php_shmop *shmop = (struct php_shmop *)rsrc->ptr;
+	php_shmop *shmop = shmop_from_obj(object);
 
 	shmdt(shmop->addr);
-	efree(shmop);
+
+	zend_object_std_dtor(&shmop->std);
 }
-/* }}} */
 
 /* {{{ PHP_MINIT_FUNCTION
  */
 PHP_MINIT_FUNCTION(shmop)
 {
-	shm_type = zend_register_list_destructors_ex(rsclean, NULL, "shmop", module_number);
+	zend_class_entry ce;
+	INIT_CLASS_ENTRY(ce, "Shmop", class_Shmop_methods);
+	shmop_ce = zend_register_internal_class(&ce);
+	shmop_ce->ce_flags |= ZEND_ACC_FINAL;
+	shmop_ce->create_object = shmop_create_object;
+	shmop_ce->serialize = zend_class_serialize_deny;
+	shmop_ce->unserialize = zend_class_unserialize_deny;
+
+	memcpy(&shmop_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
+	shmop_object_handlers.offset = XtOffsetOf(php_shmop, std);
+	shmop_object_handlers.free_obj = shmop_free_obj;
+	shmop_object_handlers.get_constructor = shmop_get_constructor;
+	shmop_object_handlers.clone_obj = NULL;
 
 	return SUCCESS;
 }
@@ -97,12 +138,12 @@ PHP_MINFO_FUNCTION(shmop)
 }
 /* }}} */
 
-/* {{{ proto resource shmop_open(int key, string flags, int mode, int size)
+/* {{{ proto Shmop shmop_open(int key, string flags, int mode, int size)
    gets and attaches a shared memory segment */
 PHP_FUNCTION(shmop_open)
 {
 	zend_long key, mode, size;
-	struct php_shmop *shmop;
+	php_shmop *shmop;
 	struct shmid_ds shm;
 	char *flags;
 	size_t flags_len;
@@ -116,9 +157,8 @@ PHP_FUNCTION(shmop_open)
 		RETURN_FALSE;
 	}
 
-	shmop = emalloc(sizeof(struct php_shmop));
-	memset(shmop, 0, sizeof(struct php_shmop));
-
+	object_init_ex(return_value, shmop_ce);
+	shmop = Z_SHMOP_P(return_value);
 	shmop->key = key;
 	shmop->shmflg |= mode;
 
@@ -175,32 +215,30 @@ PHP_FUNCTION(shmop_open)
 	}
 
 	shmop->size = shm.shm_segsz;
+	return;
 
-	RETURN_RES(zend_register_resource(shmop, shm_type));
 err:
-	efree(shmop);
+	zend_object_release(Z_OBJ_P(return_value));
 	RETURN_FALSE;
 }
 /* }}} */
 
-/* {{{ proto string shmop_read(resource shmid, int start, int count)
+/* {{{ proto string shmop_read(Shmop shmid, int start, int count)
    reads from a shm segment */
 PHP_FUNCTION(shmop_read)
 {
 	zval *shmid;
 	zend_long start, count;
-	struct php_shmop *shmop;
+	php_shmop *shmop;
 	char *startaddr;
 	int bytes;
 	zend_string *return_string;
 
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "rll", &shmid, &start, &count) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oll", &shmid, shmop_ce, &start, &count) == FAILURE) {
 		RETURN_THROWS();
 	}
 
-	if ((shmop = (struct php_shmop *)zend_fetch_resource(Z_RES_P(shmid), "shmop", shm_type)) == NULL) {
-		RETURN_THROWS();
-	}
+	shmop = Z_SHMOP_P(shmid);
 
 	if (start < 0 || start > shmop->size) {
 		php_error_docref(NULL, E_WARNING, "Start is out of range");
@@ -221,62 +259,50 @@ PHP_FUNCTION(shmop_read)
 }
 /* }}} */
 
-/* {{{ proto void shmop_close(resource shmid)
-   closes a shared memory segment */
+/* {{{ proto void shmop_close(Shmop shmid)
+   used to close a shared memory segment; now a NOP */
 PHP_FUNCTION(shmop_close)
 {
 	zval *shmid;
-	struct php_shmop *shmop;
 
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "r", &shmid) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "O", &shmid, shmop_ce) == FAILURE) {
 		RETURN_THROWS();
 	}
-
-
-	if ((shmop = (struct php_shmop *)zend_fetch_resource(Z_RES_P(shmid), "shmop", shm_type)) == NULL) {
-		RETURN_THROWS();
-	}
-
-	zend_list_close(Z_RES_P(shmid));
 }
 /* }}} */
 
-/* {{{ proto int shmop_size(resource shmid)
+/* {{{ proto int shmop_size(Shmop shmid)
    returns the shm size */
 PHP_FUNCTION(shmop_size)
 {
 	zval *shmid;
-	struct php_shmop *shmop;
+	php_shmop *shmop;
 
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "r", &shmid) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "O", &shmid, shmop_ce) == FAILURE) {
 		RETURN_THROWS();
 	}
 
-	if ((shmop = (struct php_shmop *)zend_fetch_resource(Z_RES_P(shmid), "shmop", shm_type)) == NULL) {
-		RETURN_THROWS();
-	}
+	shmop = Z_SHMOP_P(shmid);
 
 	RETURN_LONG(shmop->size);
 }
 /* }}} */
 
-/* {{{ proto int shmop_write(resource shmid, string data, int offset)
+/* {{{ proto int shmop_write(Shmop shmid, string data, int offset)
    writes to a shared memory segment */
 PHP_FUNCTION(shmop_write)
 {
-	struct php_shmop *shmop;
+	php_shmop *shmop;
 	zend_long writesize;
 	zend_long offset;
 	zend_string *data;
 	zval *shmid;
 
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "rSl", &shmid, &data, &offset) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "OSl", &shmid, shmop_ce, &data, &offset) == FAILURE) {
 		RETURN_THROWS();
 	}
 
-	if ((shmop = (struct php_shmop *)zend_fetch_resource(Z_RES_P(shmid), "shmop", shm_type)) == NULL) {
-		RETURN_THROWS();
-	}
+	shmop = Z_SHMOP_P(shmid);
 
 	if ((shmop->shmatflg & SHM_RDONLY) == SHM_RDONLY) {
 		php_error_docref(NULL, E_WARNING, "Trying to write to a read only segment");
@@ -295,20 +321,18 @@ PHP_FUNCTION(shmop_write)
 }
 /* }}} */
 
-/* {{{ proto bool shmop_delete(resource shmid)
+/* {{{ proto bool shmop_delete(Shmop shmid)
    mark segment for deletion */
 PHP_FUNCTION(shmop_delete)
 {
 	zval *shmid;
-	struct php_shmop *shmop;
+	php_shmop *shmop;
 
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "r", &shmid) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "O", &shmid, shmop_ce) == FAILURE) {
 		RETURN_THROWS();
 	}
 
-	if ((shmop = (struct php_shmop *)zend_fetch_resource(Z_RES_P(shmid), "shmop", shm_type)) == NULL) {
-		RETURN_THROWS();
-	}
+	shmop = Z_SHMOP_P(shmid);
 
 	if (shmctl(shmop->shmid, IPC_RMID, NULL)) {
 		php_error_docref(NULL, E_WARNING, "Can't mark segment for deletion (are you the owner?)");
diff --git a/ext/shmop/shmop.stub.php b/ext/shmop/shmop.stub.php
index 92f5c212b8..c19cbc26f0 100644
--- a/ext/shmop/shmop.stub.php
+++ b/ext/shmop/shmop.stub.php
@@ -2,20 +2,17 @@
 
 /** @generate-function-entries */
 
-/** @return resource|false */
-function shmop_open(int $key, string $flags, int $mode, int $size) {}
+final class Shmop {}
 
-/** @param resource $shmid */
-function shmop_read($shmid, int $start, int $count): string|false {}
+function shmop_open(int $key, string $flags, int $mode, int $size): Shmop|false {}
 
-/** @param resource $shmid */
-function shmop_close($shmid): void {}
+function shmop_read(Shmop $shmid, int $start, int $count): string|false {}
 
-/** @param resource $shmid */
-function shmop_size($shmid): int {}
+/** @deprecated */
+function shmop_close(Shmop $shmid): void {}
 
-/** @param resource $shmid */
-function shmop_write($shmid, string $data, int $offset): int|false {}
+function shmop_size(Shmop $shmid): int {}
 
-/** @param resource $shmid */
-function shmop_delete($shmid): bool {}
+function shmop_write(Shmop $shmid, string $data, int $offset): int|false {}
+
+function shmop_delete(Shmop $shmid): bool {}
diff --git a/ext/shmop/shmop_arginfo.h b/ext/shmop/shmop_arginfo.h
index 39f8615783..55914efb8c 100644
--- a/ext/shmop/shmop_arginfo.h
+++ b/ext/shmop/shmop_arginfo.h
@@ -1,7 +1,7 @@
 /* This is a generated file, edit the .stub.php file instead.
  * Stub hash: a2e7d50e79d253e7136a54222346341003cc3b04 */
 
-ZEND_BEGIN_ARG_INFO_EX(arginfo_shmop_open, 0, 0, 4)
+ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_shmop_open, 0, 4, Shmop, MAY_BE_FALSE)
 	ZEND_ARG_TYPE_INFO(0, key, IS_LONG, 0)
 	ZEND_ARG_TYPE_INFO(0, flags, IS_STRING, 0)
 	ZEND_ARG_TYPE_INFO(0, mode, IS_LONG, 0)
@@ -9,27 +9,27 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_shmop_open, 0, 0, 4)
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_shmop_read, 0, 3, MAY_BE_STRING|MAY_BE_FALSE)
-	ZEND_ARG_INFO(0, shmid)
+	ZEND_ARG_OBJ_INFO(0, shmid, Shmop, 0)
 	ZEND_ARG_TYPE_INFO(0, start, IS_LONG, 0)
 	ZEND_ARG_TYPE_INFO(0, count, IS_LONG, 0)
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_shmop_close, 0, 1, IS_VOID, 0)
-	ZEND_ARG_INFO(0, shmid)
+	ZEND_ARG_OBJ_INFO(0, shmid, Shmop, 0)
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_shmop_size, 0, 1, IS_LONG, 0)
-	ZEND_ARG_INFO(0, shmid)
+	ZEND_ARG_OBJ_INFO(0, shmid, Shmop, 0)
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_shmop_write, 0, 3, MAY_BE_LONG|MAY_BE_FALSE)
-	ZEND_ARG_INFO(0, shmid)
+	ZEND_ARG_OBJ_INFO(0, shmid, Shmop, 0)
 	ZEND_ARG_TYPE_INFO(0, data, IS_STRING, 0)
 	ZEND_ARG_TYPE_INFO(0, offset, IS_LONG, 0)
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_shmop_delete, 0, 1, _IS_BOOL, 0)
-	ZEND_ARG_INFO(0, shmid)
+	ZEND_ARG_OBJ_INFO(0, shmid, Shmop, 0)
 ZEND_END_ARG_INFO()
 
 
@@ -44,9 +44,14 @@ ZEND_FUNCTION(shmop_delete);
 static const zend_function_entry ext_functions[] = {
 	ZEND_FE(shmop_open, arginfo_shmop_open)
 	ZEND_FE(shmop_read, arginfo_shmop_read)
-	ZEND_FE(shmop_close, arginfo_shmop_close)
+	ZEND_DEP_FE(shmop_close, arginfo_shmop_close)
 	ZEND_FE(shmop_size, arginfo_shmop_size)
 	ZEND_FE(shmop_write, arginfo_shmop_write)
 	ZEND_FE(shmop_delete, arginfo_shmop_delete)
 	ZEND_FE_END
 };
+
+
+static const zend_function_entry class_Shmop_methods[] = {
+	ZEND_FE_END
+};
diff --git a/ext/shmop/tests/001.phpt b/ext/shmop/tests/001.phpt
index 48e722d2f0..55e8476b84 100644
--- a/ext/shmop/tests/001.phpt
+++ b/ext/shmop/tests/001.phpt
@@ -32,8 +32,6 @@ shmop extension test
 
     echo "data in memory is: " . shmop_read($shm_id, 0, $written) . "\n";
 
-    shmop_close($shm_id);
-
     echo "shm open for read only: ";
     $shm_id = shmop_open($hex_shm_id, "a", 0644, 1024);
     if (!$shm_id) {
@@ -47,8 +45,6 @@ shmop extension test
     /* try to append data to the shared memory segment, this should fail */
     shmop_write($shm_id, $write_d1, $written);
 
-    shmop_close($shm_id);
-
     echo "shm open for read only: ";
     $shm_id = shmop_open($hex_shm_id, "w", 0644, 1024);
     if (!$shm_id) {
@@ -73,8 +69,6 @@ shmop extension test
     } else {
         echo "ok\n";
     }
-
-    shmop_close($shm_id);
 ?>
 --EXPECTF--
 shm open for create: ok
diff --git a/ext/shmop/tests/shmop_open_private.phpt b/ext/shmop/tests/shmop_open_private.phpt
index df969885c9..f7e3a4666a 100644
--- a/ext/shmop/tests/shmop_open_private.phpt
+++ b/ext/shmop/tests/shmop_open_private.phpt
@@ -15,9 +15,6 @@ $shm2 = shmop_open(0, 'c', 0777, 1024);
 $read = shmop_read($shm2, 0, 4);
 
 var_dump(is_string($read) && $read !== $write);
-
-shmop_close($shm1);
-shmop_close($shm2);
 ?>
 --EXPECT--
 bool(true)