Bug 210916

Summary: Regression after r260359 ([GTK][WPE] lowWatermarkPages() in MemoryPressureMonitor.cpp only searches the "low" value inside the first "Node" section)
Product: WebKit Reporter: Pablo Saavedra <psaavedra>
Component: WebKitGTKAssignee: Adrian Perez <aperez>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, bugs-noreply, clopez, mcrha
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=210345
Attachments:
Description Flags
Patch
none
Patch v2 none

Description Pablo Saavedra 2020-04-23 08:56:45 PDT
Changes in https://bugs.webkit.org/show_bug.cgi?id=210345 leaks file descriptors:


```
This change is leaking file handles. I see constant additions of the triple:

  /proc/meminfo
  /proc/zoneinfo
  /proc/18738/cgroup

in `lsof -p PID`. It lasts up to the top opened files limit is reached.

You should, in MemoryPressureMonitor::start(), fclose() those which had been opened, both in the 'continue' and in the end of the while() (not only after it, which handles the 'break' path).
```

Ref: https://bugs.webkit.org/show_bug.cgi?id=210345#c5
Comment 1 Adrian Perez 2020-04-23 10:08:58 PDT
Created attachment 397355 [details]
Patch
Comment 2 Carlos Alberto Lopez Perez 2020-04-23 10:25:43 PDT
Comment on attachment 397355 [details]
Patch

interesting patch, looks good... but there are some build errors on the EWS.
Comment 3 Adrian Perez 2020-04-23 12:18:44 PDT
Created attachment 397373 [details]
Patch v2
Comment 4 Adrian Perez 2020-04-23 12:26:40 PDT
(In reply to Carlos Alberto Lopez Perez from comment #2)
> Comment on attachment 397355 [details]
> Patch
> 
> interesting patch, looks good... but there are some build errors on the EWS.

Yes, I forgot to add one bit before uploading, it should be fixed now.
Comment 5 Carlos Alberto Lopez Perez 2020-04-23 12:36:58 PDT
Comment on attachment 397373 [details]
Patch v2

r=me
Looks nice! thanks
Comment 6 EWS 2020-04-23 13:06:30 PDT
Committed r260592: <https://trac.webkit.org/changeset/260592>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 397373 [details].
Comment 7 Milan Crha 2020-04-23 23:36:36 PDT
> precise if thy are eventually

s/thy/they/

From the working point of view: seems to work properly. Thanks.