Uploaded image for project: 'PHP Driver: Extension'
  1. PHP Driver: Extension
  2. PHPC-1598

Objects with get_properties handlers should have get_gc delegate to zend_std_get_properties

    • Type: Icon: Bug Bug
    • Resolution: Fixed
    • Priority: Icon: Major - P3 Major - P3
    • 1.8.0-beta2, 1.8.0
    • Affects Version/s: 1.5.0
    • Component/s: None
    • None

      As originally reported on GitHub as #1124, ReadPreference.c may cause a segfault during shutdown/GC. The crash can be reproduced using the sample project that was attached to the original issue.


      PHPC-850 introduced get_properties handlers for ReadConcern, ReadPreference, and WriteConcern classes, which return an internally cached HashTable with properties for debugging and serialization. This was previously done for BSON classes in PHPC-460 (revised in PHPC-894 and PHPC-939). No other driver classes define get_properties handlers, per PHPC-1023.

      In the BSON patches, we implemented get_gc handler that returns a pointer to the internally cached HashTable (note: this could be null if get_properties was never called previously). I don't believe that implementation was correct, as the data in get_properties originates from internal BSON structures instead of zvals, and thus isn't relevant to GC. Prior art in core PHP extensions suggests that get_gc should return zend_std_get_properties(object).

      Delegating to zend_std_get_properties also ensures that any userland-assigned (public) properties would get passed on to the garbage collector for examination – whereas they are currently not in our present implementation. Granted, this is probably an uncommon case.

      The suggested changes for this ticket are as follows:

      • Create regression tests for segfaults during GC for all classes implementing get_properties handlers. This covers RC, RP, WC, and BSON classes, although we don't expect the BSON classes to fail as-is.
      • Modify the BSON get_gc handlers to return zend_std_get_properties(object) instead of the internally cached HashTable.
      • Define get_gc handlers for the RC, RP, and WC objects.

            Assignee:
            jmikola@mongodb.com Jeremy Mikola
            Reporter:
            andreas.braun@mongodb.com Andreas Braun
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: