D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 14573 - [REG2.067] Extreme memory usage when `synchronized( object )` is used
Summary: [REG2.067] Extreme memory usage when `synchronized( object )` is used
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All Linux
: P1 regression
Assignee: Martin Nowak
URL:
Keywords: pull
Depends on:
Blocks:
 
Reported: 2015-05-11 10:55 UTC by tcak2
Modified: 2017-07-19 17:41 UTC (History)
3 users (show)

See Also:


Attachments
Example code (404 bytes, application/x-dsrc)
2015-05-11 10:55 UTC, tcak2
Details

Note You need to log in before you can comment on or make changes to this issue.
Description tcak2 2015-05-11 10:55:22 UTC
Created attachment 1521 [details]
Example code

On DMD 2.067.1, when `synchronized` keyword is used with a parameter that is `this` or just another shared object, memory consumption increases continuously.

I am informed that on DMD 2.066.0, this is not the case, and it doesn't increase memory consumption.

As seen in the example file, if object is destroyed by using `destroy` function, even the `synchronized` is used with an object, memory usage is still stable.
Comment 2 Martin Nowak 2015-05-13 15:26:45 UTC
Sounds like the object mutexes aren't freed. They're usually explicity destroyed when finalizing a class. Might need to collect them by the GC separately.
Comment 3 safety0ff.bugz 2015-05-13 17:07:21 UTC
Probably best just to revert the commit in question for now and leave a comment in _d_newclass explaining why classes need the FINALIZE flag regardless.

The other commit in PR #873 can stay.
Comment 4 Martin Nowak 2015-05-14 17:16:57 UTC
(In reply to safety0ff.bugz from comment #3)
> Probably best just to revert the commit in question for now and leave a
> comment in _d_newclass explaining why classes need the FINALIZE flag
> regardless.

I'm so tired of this reverting business, let's just fix the bug.
We have 2 options

- introduce a finalizeMonitor flag that tells the GC to only free the monitor
- try to add a quickpath in rt_finalize2 to skip destruction, but keep the FINALIZE flag, even for classes without a dtor
Comment 5 safety0ff.bugz 2015-05-14 17:50:10 UTC
(In reply to Martin Nowak from comment #4)
> 
> I'm so tired of this reverting business, let's just fix the bug.
> We have 2 options
> 
> - introduce a finalizeMonitor flag that tells the GC to only free the monitor
> - try to add a quickpath in rt_finalize2 to skip destruction, but keep the
> FINALIZE flag, even for classes without a dtor

I think introducing another flag will be overhead for little gain (flags aren't cheap currently.)

The first idea that came to mind was to add FINALIZE in _d_monitor_create, not sure if it'll work.

I don't have the time for a fix that will require me to check the monitor source code, which is why I talked about reverting + comment.
Comment 6 Martin Nowak 2015-05-14 18:24:57 UTC
(In reply to safety0ff.bugz from comment #5)
> The first idea that came to mind was to add FINALIZE in _d_monitor_create,
> not sure if it'll work.

Interesting idea.
Comment 7 Martin Nowak 2015-05-14 18:33:22 UTC
(In reply to Martin Nowak from comment #6)
> Interesting idea.

Even better would be to pick up the old explicit @monitor idea and finally implement it.
https://github.com/D-Programming-Language/dmd/pull/3547
Comment 9 github-bugzilla 2015-05-20 12:30:41 UTC
Commit pushed to master at https://github.com/D-Programming-Language/druntime

https://github.com/D-Programming-Language/druntime/commit/d50aba0083b03659f60c3f157a674d9846d8228f
fix Issue 14573 - classes without destructor leak monitor/mutex

- set finalize bit when constructing the monitor
Comment 10 github-bugzilla 2015-06-17 21:02:51 UTC
Commit pushed to stable at https://github.com/D-Programming-Language/druntime

https://github.com/D-Programming-Language/druntime/commit/d50aba0083b03659f60c3f157a674d9846d8228f
fix Issue 14573 - classes without destructor leak monitor/mutex
Comment 11 github-bugzilla 2017-07-19 17:41:41 UTC
Commit pushed to dmd-cxx at https://github.com/dlang/druntime

https://github.com/dlang/druntime/commit/d50aba0083b03659f60c3f157a674d9846d8228f
fix Issue 14573 - classes without destructor leak monitor/mutex