[BUG]: Segmentation Fault Resultset/Complex, keepSnapshots=true #14783
Comments
|
Thnx for reporting @maxc62 We're going to investigate this. |
|
@maxc62 Currently we're unable to reproduce your issue. Can you have a look at our test? Did we missed something? https://github.com/phalcon/cphalcon/pull/14825/files#diff-9dd2e736386869acb3d7b6c87d328974R62 |
|
@ruudboon private function transform($resultset)
{
$customer = $resultset->readAttribute(lcfirst(CustomersKeepSnapshots::class));
$invoices = $resultset->readAttribute('join_1');
$customer->invoices = $invoices;
return $customer;
} |
|
You need also to change "mvcModelQueryIssue14783" foreach ($resultsets as $resultset) {
$model = $this->transform($resultset);
$I->assertInstanceOf(CustomersKeepSnapshots::class, $model);
$I->assertInstanceOf(InvoicesKeepSnapshots::class, $model->invoices);
} |
|
@maxc62 I adjusted the tests but wasn't able to reproduce it locally. Do you have the possibility to clone the repo, switch to 4.0.x (after pull merge) and run the tests on your system? If you have a local MySql server running you can also adjust the .env and skip the docker-compose part. |
|
I'm having a little trouble running your test. Finally I manage to test it, but this one seems to pass. So I created a single page with all the code on it. DROP TABLE IF EXISTS co_customers;
create table co_customers
(
cst_id serial not null primary key,
cst_status_flag smallint,
cst_name_last varchar(100),
cst_name_first varchar(100)
);
alter table co_customers owner to phalcon;
DROP TABLE IF EXISTS co_invoices;
create table co_invoices
(
inv_id serial not null constraint co_invoices_pk primary key,
inv_cst_id integer,
inv_status_flag smallint,
inv_title varchar(100),
inv_total numeric(10, 2),
inv_created_at timestamp
);
alter table co_invoices owner to phalcon;
INSERT INTO co_customers (cst_id, cst_status_flag, cst_name_last, cst_name_first)
SELECT generate_series, 1, 'last' || generate_series::text, 'first' || generate_series::text FROM generate_series(1, 50);
INSERT INTO co_invoices (inv_cst_id, inv_status_flag, inv_title)
SELECT generate_series, 1, 'title' || generate_series::text FROM generate_series(1, 50);
And the php code <?php
use Phalcon\Di\FactoryDefault;
use Phalcon\Mvc\Model;
$di = new FactoryDefault();
$di->set('db', function () {
$adapter = new \Phalcon\Db\Adapter\Pdo\Postgresql([
'host' => '127.0.0.1',
'username' => 'phalcon',
'password' => 'phalcon',
'dbname' => 'phalcon',
'schema' => 'public'
]);
return $adapter;
});
class CustomersKeepSnapshots extends Model
{
public $cst_id;
public $cst_status_flag;
public $cst_name_last;
public $cst_name_first;
public function initialize()
{
$this->keepSnapshots(true);
$this->setSource('co_customers');
$this->belongsTo(
'cst_id',
'InvoicesKeepSnapshots',
'inv_cst_id',
[
'alias' => 'invoices'
]
);
}
}
class InvoicesKeepSnapshots extends Model
{
public $inv_id;
public $inv_cst_id;
public $inv_status_flag;
public $inv_title;
public $inv_total;
public $inv_created_at;
private $secretValue;
private $superSecret;
public function initialize()
{
$this->setSource('co_invoices');
$this->keepSnapshots(true);
}
}
function transform($resultset)
{
$customer = $resultset->readAttribute(lcfirst(CustomersKeepSnapshots::class));
$invoices = $resultset->readAttribute('join_1');
$customer->invoices = $invoices;
return $customer;
}
$query = CustomersKeepSnapshots::query();
$query->columns(
[
CustomersKeepSnapshots::class . '.*',
'join_1.*'
]
);
$query->leftJoin(
InvoicesKeepSnapshots::class,
'join_1.inv_cst_id = ' . CustomersKeepSnapshots::class . '.cst_id',
'join_1'
);
$query->limit(20, 0);//I have 50 rows in my db
$resultsets = $query->execute();
foreach ($resultsets as $resultset) {
$model = transform($resultset);
}
echo 'OK';
It's strange, I test this code with 2 servers with up to date php Thank you |
Adjusted test according to feedback (#14783)
|
@maxc62 Great this works! ....well... I'm able to reproduce a segfault ;) Thnx for the extended info etc. I will try to dive into this tomorrow. |
|
Hi, class CustomersKeepSnapshots extends Model
{
public $cst_id;
public $cst_status_flag;
public $cst_name_last;
public $cst_name_first;
public function initialize()
{
$this->keepSnapshots(true);
$this->setSource('co_customers');
$this->belongsTo(
'cst_id',
'InvoicesKeepSnapshots',
'inv_cst_id',
[
'alias' => 'invoices'
]
);
}
public function __set($property, $value)
{
/**
* Values are probably relationships if they are objects
*/
if (is_object($value) && $value instanceof ModelInterface) {
$lowerProperty = strtolower($property);
$modelName = get_class($this);
$manager = $this->getModelsManager();
$relation = $manager->getRelationByAlias(
$modelName,
$lowerProperty
);
if (is_object($relation)) {
$dirtyState = $this->dirtyState;
if ($value->getDirtyState() != $dirtyState) {
$dirtyState = self::DIRTY_STATE_TRANSIENT;
}
unset($this->related[$lowerProperty]);
$this->dirtyRelated[$lowerProperty] = $value;
$this->dirtyState = $dirtyState;
return $value;
}
}
/**
* Check if the value is an array
*/
elseif (is_array($value)) {
$lowerProperty = strtolower($property);
$modelName = get_class($this);
$manager = $this->getModelsManager();
$relation = $manager->getRelationByAlias(
$modelName,
$lowerProperty
);
if (is_object($relation)) {
switch ($relation->getType()) {
case Relation::BELONGS_TO:
case Relation::HAS_ONE:
/**
* Load referenced model from local cache if its possible
*/
$referencedModel = $manager->load(
$relation->getReferencedModel()
);
if (is_object($referencedModel)) {
$referencedModel->assign($value);
unset($this->related[$lowerProperty]);
$this->dirtyRelated[$lowerProperty] = $referencedModel;
$this->dirtyState = self::DIRTY_STATE_TRANSIENT;
return $value;
}
break;
case Relation::HAS_MANY:
case Relation::HAS_MANY_THROUGH:
$related = [];
foreach($value as $item) {
if (is_object($item)) {
if ($item instanceof ModelInterface) {
$related[] = $item;
}
}
}
if (count($related) > 0) {
unset($this->related[$lowerProperty]);
$this->dirtyRelated[$lowerProperty] = $related;
$this->dirtyState = self::DIRTY_STATE_TRANSIENT;
return $value;
}
break;
}
}
}
// Use possible setter.
if ($this->_possibleSetter($property, $value)) {
return $value;
}
/**
* Throw an exception if there is an attempt to set a non-public
* property.
*/
if (property_exists($this, $property)) {
$manager = $this->getModelsManager();
if (!$manager->isVisibleModelProperty($this, $property)) {
throw new Exception(
"Cannot access property '" . $property . "' (not public)."
);
}
}
$this->{$property} = $value;
return $value;
}
} |
|
@ruudboon In method "__set", I added a test before deleting a variable that didn't exist. if typeof relation == "object" {
let dirtyState = this->dirtyState;
if (value->getDirtyState() != dirtyState) {
let dirtyState = self::DIRTY_STATE_TRANSIENT;
}
if isset this->related[lowerProperty] {
unset this->related[lowerProperty];
}
let this->dirtyRelated[lowerProperty] = value,
this->dirtyState = dirtyState;
return value;
}With this modification the problem no longer exists I've checked all the unset calls in phalcon and there's very little use of unset with an "unset($obj->var)". In Acl/Adapter/Memory.zep, method "dropComponentAccess": already test, before unset if isset this->accessList[accessKey] {
unset this->accessList[accessKey];
}In Events/Manager.zep, method "detachAll": already test, before unset if isset this->events[type] {
unset this->events[type];
}In Filter.zep, method "set": maybe the property services are assigned beforehand? public function set(string! name, callable service) -> void
{
let this->mapper[name] = service;
unset this->services[name];
}In Events/Form.zep, method "remove": already test, before unset if isset this->elements[name] {
unset this->elements[name];
return true;
}In Factory/AbstractFactory.zep method "init": maybe the property services are assigned beforehand? protected function init(array! services = []) -> void
{
var adapters, name, service;
let adapters = this->getAdapters(),
adapters = array_merge(adapters, services);
for name, service in adapters {
let this->mapper[name] = service;
unset(this->services[name]);
}
}And in our case Mvc/Model.zep, method "__set": if typeof relation == "object" {
let dirtyState = this->dirtyState;
if (value->getDirtyState() != dirtyState) {
let dirtyState = self::DIRTY_STATE_TRANSIENT;
}
unset this->related[lowerProperty];
let this->dirtyRelated[lowerProperty] = value,
this->dirtyState = dirtyState;
return value;
}And with an array, I tried this case by modifying our test by $customer->invoices = [$invoices];I got the same segmentation error. With the same patch, no problem (be careful twice here) case Relation::HAS_ONE:
/**
* Load referenced model from local cache if its possible
*/
let referencedModel = manager->load(
relation->getReferencedModel()
);
if typeof referencedModel == "object" {
referencedModel->assign(value);
unset this->related[lowerProperty];
let this->dirtyRelated[lowerProperty] = referencedModel,
this->dirtyState = self::DIRTY_STATE_TRANSIENT;
return value;
}
break;
case Relation::HAS_MANY:
case Relation::HAS_MANY_THROUGH:
let related = [];
for item in value {
if typeof item == "object" {
if item instanceof ModelInterface {
let related[] = item;
}
}
}
if count(related) > 0 {
unset this->related[lowerProperty];
let this->dirtyRelated[lowerProperty] = related,
this->dirtyState = self::DIRTY_STATE_TRANSIENT;
return value;
}
break;I had also tried with this test: $customer->invoices;
$customer->invoices = [$invoices];And this case worked, so the problem does come from unset() I think there's an error in zephir using "unset($obj->var);" I will try to see if it is possible to get the problem in the "set" method of Filter.zep and "init" of Factory/AbstractFactory.zep |
Jeckerson
added
4.1.1
Jeckerson
added
5.0
|
This was fixed in |
Jeckerson
added
the
external dependency
Describe the bug
When I read model attribute on Resultset\Complex and affect it in another model, I get segmentation fault
To Reproduce
Create Model1:
Create Model2:
IndexController
Expected behavior
Display "OK"
Test
// $this->keepSnapshots(true);Additional context
I try with php 7.3 & php 7.4 with phalcon 4 and same result (Failed)
With php 7.3 or php 7.2 and phalcon 3 (last version), it works
Details
Backtrace PHP 7.4
zBacktrace PHP 7.4
The text was updated successfully, but these errors were encountered: