Merged.

The quickfix proposed by Silvan in msg10806 was implemented to get rid of the segfault. The underlying issue is still unresolved and further discussed in issue1065.

I created issue1065 for the underlying problem and copied the nosy list from here. Of course feel free to un-nosy yourself if you are not interested.

Probably yes. The way I read the discussion, we hope that with the new component interaction model this will have to be addressed necessarily, but it's probably good to have an explicit record of this problem regardless to inform whatever decisions we make at that point.

I'm fine with merging the change like this as a hotfix.

But the underlying issue is not really solved by this. The log in the information object still becomes a dangling reference when the pattern generator is deleted. With the patch, we just do not access it anymore afterwards. Should we create a follow-up issue for a better solution?

I have nothing to add.

Looks good to me.

I'd write the change log entry differently -- focusing on the user perspective, not the implementation details. It's enough to say that we fixed a crash there without mentioning how we changed the code to do it. But we always polish these when we do a release, so for me this looks good to merge.

Can one of the people that know more about this (perhaps Silvan?) also give a go-ahead?

I have opened a pull request that implements the change proposed by Silvan in msg10806. The pull request can be found here:
https://github.com/aibasel/downward/pull/121

The builder solution will at least force to think about how the log, if the information class should still have one, gets there - be it by moving from the generator, creating their own copy or something else.

I'm not sure how this interacts with the component interaction problem but it sounds like exactly the kind of problem that we discussed there. Would the problem go away after we switch everything to builders? If so, i think a short term solution like swapping the two lines would be sufficient for now.

You are absolutely right, the problem is that the log of the information is a reference to the log of the generator. The pho code apparently is the only one deleting the generator after using it, see the following lines in pho_constraints.cc:

...
pattern_generator = nullptr;
pdbs = pattern_collection_info.get_pdbs();
...

Switching these lines would fix this particular error, but of course, this error will always occur when deleting the generator and still using the result of the generator, i.e., the information object. 

We could fix this by moving the log out of the generator, thus making the generator unusable afterwards. Currently, all generators produce results stored in shared pointers, handed to the information classes. Still, I'm sure the generators couldn't just be re-used without some effort after having them used once. 

We could also "share" the log like everything else which is produced by generators and handed to information classes, but as advertised since years (c.f. issue906, and also related, issue911), I'm really against this. I think I likely wouldn't have made this error if the information classes where transferring ownership, rather than keeping shared pointers to everything created from the generators. The generators should be one-time usable factories that create everything they want and then go out of scope.

Another solution could be to stop producing output in the information classes. This would probably be the easiest fix for the time being.

Silvan, I think you had contact with the logs and the pattern generation method. Could it be that the logger somehow goes out of scope and is destroyed? Of course it could also be a memory corruption at a different place in the code.

Using cpdbs with the same pattern collection works fine, so it has to be something related to the pho constraints. Maybe cpdbs create the PDBs earlier and the pho constraints delay the construction to a point where something is no longer available.

This call lead to a segfault
./fast-downward.py $DOWNWARD_BENCHMARKS/blocks/probBLOCKS-4-0.pddl --search "eager_greedy([operatorcounting([pho_constraints(patterns=systematic(2))])])"

GDB shows the stacktrace in this line https://github.com/aibasel/downward/blob/main/src/search/pdbs/pattern_collection_information.cc#L72
printing the char[45] (so not the timer in the line below) to the log. This is strange because printing the text in line 63 works fine.