Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

problems with memory management of barriers #15116

Open
mppf opened this issue Mar 6, 2020 · 2 comments
Open

problems with memory management of barriers #15116

mppf opened this issue Mar 6, 2020 · 2 comments

Comments

@mppf
Copy link
Member

mppf commented Mar 6, 2020

After PR #15041, there are failures in test/parallel/taskPar/elliot/barrier/array-of-barriers.chpl due to errors in the memory management of barriers. Memory management for barriers started in PR #4945.

Here is an example that changed behavior after PR #15041:

var barArr: [1..1] Barrier;
barArr[1] = new Barrier(1);

the new barrier is now deinited immediately after the assignment statement. The array stores a Barrier with isowned=false, which is then used in a use-after-free.

Even before that PR, the memory management strategy in Barrier (always set isowned=true in = or init=) was problematic. In particular, it is impossible to initialize some Barrier objects in a loop. For example:

use Barriers;
var ba = [i in 1..10] new Barrier(1);
for bar in ba do bar.barrier();

has use-after-free memory errors even before my PR.

How can Barrier manage memory more appropriately? (And, should Barrier be renamed to barrier ?)

Here are some possible short-term directions:

  • future-ize array-of-barriers.chpl
  • change Barrier to store an owned class field
  • make Barrier produce a compilation error if one is copied or assigned
mppf added a commit that referenced this issue Mar 6, 2020
Future-ize array-of-barriers test

This PR future-izes the test

    test/parallel/taskPar/elliot/barrier/array-of-barriers.chpl

for until #15116 is solved.

Trivial and not reviewed (but discussed with @daviditen ).
@bradcray
Copy link
Member

Is this issue expected to have an impact on users? Only if they use arrays of barriers or in general? Does it represent a backslide from the previous release? Should we be prioritizing addressing it?

@mppf
Copy link
Member Author

mppf commented Mar 17, 2020

Is this issue expected to have an impact on users? Only if they use arrays of barriers or in general? Does it represent a backslide from the previous release? Should we be prioritizing addressing it?

I don't expect users have arrays of barriers.

Does it represent a backslide from the previous release?

Arrays created in any kind of loop wouldn't have worked in previous releases.

Should we be prioritizing addressing it?

I suspect arrays of barriers are rare and the typical use case has the barrier in a variable. So I think we could choose to put it off if we like.

However I absolutely think it needs to be fixed.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants