Resource to opaque object migration

[Girgias]

A long term project is to move all extension resources to objects as they are all in all better.

This issue aims to list the remaining usages of resources within php-src

Related RFC: https://wiki.php.net/rfc/resource_to_object_conversion

PHP 8.0:

• cURL Convert CURL resources to objects php-src#5402
• enchant php/php-src@cd3e04d
• GD, GdImage [WIP] GD Resources -> Objects php-src#4714
• OpenSSL Convert OpenSSL resources to objects php-src#5860
  • OpenSSL X.509
  • OpenSSL X.509 CSR
  • OpenSSL key
• shmop Convert shmop resources to opaque objects php-src#5537
• sockets Convert resources to objects in ext/sockets php-src#5900
  • Socket
  • AddressInfo
• sysvmsg Convert resource to object in Sysvmsg php-src#5546
• sysvsem Convert resource to object in ext/sysvsem php-src#5508
• sysvshm Convert resource to object in Sysvshm extension php-src#5499
• XML - Convert ext/xml to use object instead of resource php-src#3526
• XML-RPC Convert resource to object in XML-RPC extension php-src#5457
• XMLWriter - Change XMLWriter resources to objects php-src#4706
• ZIP Deprecate the procedural API of ext/zip php-src#5746
• ZLIB: Convert resources to objects in ext/zlib php-src#5680

PHP 8.1:

• fileinfo Convert file_info resources to objects php-src#5987
• FTP: le_ftpbuf Convert FTP connection resource to an opaque object php-src#5945
• GD: le_gd_font php/php-src@bc40bce
• pspell: le_pspell, le_pspell_config php/php-src@bd12c94
• IMAP: le_imap Convert IMAP resource to object php-src#6418
• LDAP: le_link, le_result, le_result_entry (Convert resources to objects in ext/ldap php-src#6770)
• pgsql: le_link, le_lofp (Convert resources to objects in ext/pgsql php-src#6791) - le_string (php/php-src@f26047f)

Scheduled for PHP 8.4:

• ODBC: le_result, le_conn, lepconn (Migrate ext/odbc resources to opaque objects php-src#12040)
• SOAP: le_sdl, le_url, le_typemap (Migrate ext/soap resources to objects php-src#14121, Migrate SOAP table resource to array php-src#14174)
• DBA: le_db, le_pdb (Migrate ext/dba resources to objects php-src#14239)
• com: le_istream, le_dispatch (No ext/com-dotnet resources php-src#14282)

Scheduled for PHP 9.0:

• proc: le_proc_open (Migrate resource returned by proc_open() to opaque object php-src#12098)

Remaining:

• stream (main/, ext/standard/) [libxml_set_streams_context() is also affected]
  • file/dir streams: le_stream / le_pstream
  • context: le_stream_context
  • filter: le_stream_filter
  • Bucket API: le_bucket / le_bucket_brigade

Persistent resources that are only used internally and not exposed to userland:

• MySQLi: le_pmysqli
• PDO: le_ppdo
• PGSQL: le_plink
• ODBC: le_pconn
• DBA: le_pdb

[remicollet]

About ZIP, I don't know if it worth the work, as we already have a full OO API.
Perhaps we should rather deprecate the old functional and minimal API.

[kocsismate]

@remicollet I was wondering about this a while ago. See my question at php/php-src#5601 (comment)

I even had a try at the conversion, but for me, it didn't go as easy as it seemed first, so I stopped. As far as I remember, my main problem was that the data structures used by the OO and procedural interface don't align really well.

Is it really the case? If so, then I'd also prefer deprecating the old API.

[Ayesh]

I apologize if this was discussed before.

Wouldn't it be appropriate to declare a ResourceInterface and have all the classes implement that? Methods such as ResourceInterface::close can provide a unified way to close resources, ResourceInterface::getId can return spl_object_id()-esque results, and serializers for example can refuse to serialize all \ResourceInterface instances.

[Girgias]

I apologize if this was discussed before.

Wouldn't it be appropriate to declare a ResourceInterface and have all the classes implement that? Methods such as ResourceInterface::close can provide a unified way to close resources, ResourceInterface::getId can return spl_object_id()-esque results, and serializers for example can refuse to serialize all \ResourceInterface instances.

The whole point of the migration is to move away from the legacy resource behaviour, moreover with objects all the x_close variants become no-ops.

Adding an OOP API to the opaque objects is also a completely orthogonal issue to moving resources to opaque objects.

And I'd rather that we have a long think about each individual API instead of just bundling a weird interface which will be mostly useless IMHO

[Ayesh]

With the curl class implementation, for example, is_resource() returns false for CurlHandle class objects. This means anyone who wants to check if a given variable is a resource (in a semantic sense) needs to use is_resource and instanceOf.

Having an interface means we will need only one instaceOf op, as opposed to every different class name the migration brings.

[cmb69]

This means anyone who wants to check if a given variable is a resource […]

Why do you want to check this? Is there any reason not to check for !== false?

[nikic]

Because this is an anticipated failure mode of hosting this internal coordination tracker on GitHub, I'm going to err on the side of over-moderation and say: This is not the place to have this discussion. These issues have been discussed extensively on the PHP internals list in the past, and that would also be the place to revisit any decisions.

[sgolemon]

BZ2 should be filed under ext/standard/streams
The only resources it interacts with are streams.

[sgolemon]

Regarding streams (which are going to be biggest PITA by far), I've been thinking it might make transition smoother (and make calling semantics generally nicer for extension authors) to add a new ZPP type F which returns a php_strea*, so:

  php_stream *stream;

  if (zend_parse_parameters(ZEND_NUM_ARGS(), "F", &stream) == FAILURE) {
    RETURN_THROWS();
  }

The idea being that we can update the rest of the runtime where streams are taken as arguments to remove any remaining hint of resources first, then work on the internals without a massive chunk of the userspace API breaking underneath us. Obviously a handful of functions will still need special attention (fclose(), a few stream_*() functions), but they'll be the minority.

We could even update the stubs file with a not-yet-extant "File" class and initially have the arginfo generator regard this as ZEND_ARG_TYPED_INFO(IS_RESOURCE), but when we do the big switchover, just have the generator now use ZEND_ARG_OBJ_INFO(File). Again, this should let us focus on the hard parts without being bogged down by a thousand callsites.

Edit to add: I just remembered why I never moved on this. php_stream is a PHP thing, Zend shouldn't call back to PHP. Back to the drawing board.

[sgolemon]

Okay, expanded idea:

typedef void* (zend_parse_param_callback*)(zval *arg);

// In zpp arg handler:
    case 'R': { /* Resolver */
        void **ptr = va_arg(*va, void **);
        zend_parse_param_callback *callback = va_arg(*va, zend_parse_param_callback*);

        *ptr = callback(arg);
        if (!*ptr) {
            ZEND_ASSERT(EG(exception));
            return "";
       }
       break;
    }

Then callsites look like:

{
    php_stream *stream;

    if (zend_parse_parameters(ZEND_NUM_ARGS(), "R", &stream, php_stream_from_zval) == FAILURE) {
        RETURN_THROWS();
    }
}

Edit to add: While php_stream_from_zval() doesn't currently throw exceptions, I'm sure we could use a version which does throw a proper TypeError.

[sgolemon]

On the other hand.... I've just actually sat down to do grep -r php_stream_to_zval ext/ | wc -l and the answer is only 60. So I may be making a tempest in a teapot.