View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001686 | OpenFOAM | Bug | public | 2015-05-07 15:46 | 2015-05-08 08:35 |
| Reporter | chriss85 | Assigned To | henry | ||
| Priority | normal | Severity | major | Reproducibility | sometimes |
| Status | closed | Resolution | unable to reproduce | ||
| Platform | GNU/Linux | OS | Ubuntu | OS Version | 14.04 |
| Product Version | dev | ||||
| Summary | 0001686: Memory management errors in laminar.C and turbulenceModel.H | ||||
| Description | I think there are memory management errors between compressible::turbulenceModel and compressible::RASModels:laminar. In short, the laminar class returns tmp<volScalarField> objects for all kinds of properties. This means that these objects will be destroyed when there is no more tmp object. The turbulenceModel class returns the boundary field patches for these properties in multiple functions without keeping the tmp object. This can result in errors of the use-after-free type. I also wrote a forum post about this at http://www.cfd-online.com/Forums/openfoam-programming-development/152735-understanding-memory-handling-geometricfields-posible-bug.html#post545432 | ||||
| Steps To Reproduce | In my case it happens sometimes when calling turb.kappaEff(patchI) with a valid patch index. It will either happen reproducibly for me when I rerun the program directly or not at all. Changing some unrelated stuff and recompiling has an influence on this as well, so this points to a memory management error. | ||||
| Additional Information | I suggest storing the tmp<volScalarField> objects in the laminar.C class instead of using tmp<volScalarField> objects which are only valid during the function call. This way, the volScalarField won't be destroyed and the child patch fields will remain valid. I have tested this locally for the case I described above and so far I have not seen the crash again. This is likely also a problem for the other properties in laminar.C but I have not tested this yet. It's also possible that other classes of the thermophysical models or turbulence models are affected if they use a similar structure so a larger review might be needed. | ||||
| Tags | No tags attached. | ||||
|
|
Just saw https://github.com/OpenFOAM/OpenFOAM-2.3.x/commit/ab7c7a7d1c2270d384cea8eda80d11fcc403e316 . I think this might be a fix to the problem I reported but I have not checked out the latest version yet. However, judging from my first glance I think this will only correct the effects and not the cause. |
|
|
Returning a tmp wrapper around a non-tmp non-pointer volField does not allow for the field to be deleted when the tmp is destroyed. Also given that the return type is a tmp it should not be assumed on the receiving-end that the object is in fact not temporary; it could be either. I am not aware of any memory management issues in OpenFOAM-dev relating to any of the classes you refer to. If you could provide a test which reproduces a problem either by crash or reported by valgrind it would help a lot. |
|
|
I'm sorry that I did not explain my proposed solution better. I created the volScalarField on the heap and store it in a tmp object in the laminar class. This way it remains in memory as long as the laminar class exists and it's boundary may be safely returned because the volScalarField remains valid. I will try to produce a test case as soon as I find the time. |
|
|
I understand your proposal but it is not be necessary if the tmps returned from the access functions are handled correctly as they should be. The issues I have found in the past relate to boundary conditions not holding the returned tmps for the duration of the use of the data and holding references to components of the tmp after the tmp goes out of scope. I am not aware of any more bugs of this type but if you find any please report and I will fix them. If you suspect a bug of this type but the code crash is intermittent try running with valgrind. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2015-05-07 15:46 | chriss85 | New Issue | |
| 2015-05-07 15:55 | chriss85 | Note Added: 0004724 | |
| 2015-05-07 16:07 | henry | Note Added: 0004725 | |
| 2015-05-08 08:27 | chriss85 | Note Added: 0004731 | |
| 2015-05-08 08:34 | henry | Note Added: 0004732 | |
| 2015-05-08 08:34 | henry | Status | new => closed |
| 2015-05-08 08:35 | henry | Assigned To | => henry |
| 2015-05-08 08:35 | henry | Resolution | open => unable to reproduce |
