Skip to content

Commit ea72d78

Browse files
TruongQuangSBTruongQuangSB
authored
Fix Memory leak (#1931)
* Fix Memory leak * Update Junit test
1 parent f9ea7f3 commit ea72d78

File tree

8 files changed

+104
-57
lines changed

8 files changed

+104
-57
lines changed

‎java/bundles/org.eclipse.set.application/src/org/eclipse/set/application/controlarea/ControlAreaSelectionControl.java‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.eclipse.set.basis.constants.ContainerType;
3636
import org.eclipse.set.basis.constants.Events;
3737
import org.eclipse.set.basis.constants.TableType;
38+
import org.eclipse.set.basis.files.ToolboxFileRole;
3839
import org.eclipse.set.core.services.part.ToolboxPartService;
3940
import org.eclipse.set.model.planpro.Ansteuerung_Element.Stell_Bereich;
4041
import org.eclipse.set.ppmodel.extensions.PlanProSchnittstelleExtensions;
@@ -46,6 +47,7 @@
4647
import org.eclipse.set.utils.events.ToolboxEventHandler;
4748
import org.eclipse.set.utils.events.ToolboxEvents;
4849
import org.eclipse.swt.widgets.Composite;
50+
import org.osgi.service.event.Event;
4951
import org.slf4j.Logger;
5052
import org.slf4j.LoggerFactory;
5153

@@ -105,9 +107,10 @@ public ControlAreaSelectionControl(final Composite parent,
105107
broker = serviceProvider.broker;
106108
messages = serviceProvider.messages;
107109
partService = serviceProvider.partService;
108-
// Reset combo value, when close session
109-
broker.subscribe(Events.CLOSE_SESSION, event -> initCombo());
110110
createCombo(parent);
111+
// Reset combo value, when close session
112+
broker.subscribe(Events.CLOSE_SESSION, this::closeSession);
113+
broker.subscribe(Events.MODEL_CHANGED, this::sessionChanged);
111114
}
112115

113116
private void createCombo(final Composite parent) {
@@ -127,8 +130,6 @@ public void accept(final NewTableTypeEvent t) {
127130
t.getTableType());
128131
}
129132
};
130-
ToolboxEvents.subscribe(broker, NewTableTypeEvent.class,
131-
newTableTypeHandler);
132133

133134
// register for session changes
134135
application.getContext().runAndTrack(new RunAndTrack() {
@@ -381,4 +382,25 @@ private void handleStringValue(final String msg) {
381382
}
382383
}
383384
}
385+
386+
private void closeSession(final Event event) {
387+
final Object property = event.getProperty(IEventBroker.DATA);
388+
if (property instanceof final ToolboxFileRole role
389+
&& role.equals(ToolboxFileRole.SESSION)) {
390+
ToolboxEvents.unsubscribe(broker, newTableTypeHandler);
391+
// Reset combo to default
392+
initCombo();
393+
}
394+
}
395+
396+
private void sessionChanged(final Event event) {
397+
final Object property = event.getProperty(IEventBroker.DATA);
398+
if (property instanceof final ToolboxFileRole role
399+
&& role.equals(ToolboxFileRole.SESSION)) {
400+
ToolboxEvents.subscribe(broker, NewTableTypeEvent.class,
401+
newTableTypeHandler);
402+
// Reset combo to default
403+
initCombo();
404+
}
405+
}
384406
}

‎java/bundles/org.eclipse.set.core.test/src/org/eclipse/set/core/fileservice/PlainToolboxFileTest.xtend‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class PlainToolboxFileTest extends AbstractToolboxFileTest {
2929
def void testOpen() throws Throwable{
3030
MockPlanProVersionService.mockPlanProVersionService([
3131
whenOpen
32-
thenExpectContentsExists(true)
32+
thenExpectContentsExists()
3333
])
3434
}
3535

@@ -41,7 +41,7 @@ class PlainToolboxFileTest extends AbstractToolboxFileTest {
4141
MockPlanProVersionService.mockPlanProVersionService([
4242
whenOpen
4343
whenClose
44-
thenExpectContentsExists(false)
44+
thenExpectContentsNotExists()
4545
])
4646

4747
}
@@ -55,7 +55,7 @@ class PlainToolboxFileTest extends AbstractToolboxFileTest {
5555
whenOpen
5656
whenClose
5757
whenClose
58-
thenExpectContentsExists(false)
58+
thenExpectContentsNotExists()
5959
])
6060
}
6161

@@ -68,11 +68,11 @@ class PlainToolboxFileTest extends AbstractToolboxFileTest {
6868
def void testCloseThenOpen() throws Throwable {
6969
MockPlanProVersionService.mockPlanProVersionService([
7070
whenOpen
71-
thenExpectContentsExists(true)
71+
thenExpectContentsExists()
7272
whenClose
73-
thenExpectContentsExists(false)
73+
thenExpectContentsNotExists()
7474
whenOpen
75-
thenExpectContentsExists(true)
75+
thenExpectContentsExists()
7676
])
7777
}
7878

‎java/bundles/org.eclipse.set.core.test/src/org/eclipse/set/core/fileservice/ZippedToolboxFileTest.xtend‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,18 @@
99

1010
package org.eclipse.set.core.fileservice
1111

12-
import java.lang.Throwable
1312
import java.nio.file.Files
1413
import java.nio.file.Paths
1514
import org.eclipse.set.basis.files.ToolboxFile
1615
import org.eclipse.set.basis.files.ToolboxFileRole
1716
import org.eclipse.set.core.services.files.ToolboxFileFormatService
17+
import org.eclipse.set.feature.validation.session.SetSessionService
1818
import org.eclipse.set.model.planpro.PlanPro.PlanProPackage
1919
import org.eclipse.set.unittest.utils.toolboxfile.AbstractToolboxFileTest
2020
import org.junit.jupiter.api.Test
2121

22+
import static org.junit.Assert.assertFalse
2223
import static org.junit.Assert.assertNotNull
23-
import static org.junit.Assert.assertTrue
24-
import org.eclipse.set.feature.validation.session.SetSessionService
2524

2625
/**
2726
* Test for {@link ZippedPlanProToolboxFile}
@@ -37,8 +36,8 @@ class ZippedToolboxFileTest extends AbstractToolboxFileTest {
3736
def void testOpen() throws Throwable{
3837
MockPlanProVersionService.mockPlanProVersionService([
3938
whenOpen
40-
thenExpectContentsExists(true)
41-
thenExpectLayoutContentExists(true)
39+
thenExpectContentsExists()
40+
thenExpectLayoutContentExists()
4241
thenExpectResourceCallsWithinZipDirectory
4342
])
4443
}
@@ -53,7 +52,7 @@ class ZippedToolboxFileTest extends AbstractToolboxFileTest {
5352
whenOpen
5453
whenClose
5554
thenExpectZippedDirectoryNotExist
56-
thenExpectContentsExists(false)
55+
thenExpectContentsNotExists()
5756
])
5857
}
5958

@@ -67,8 +66,7 @@ class ZippedToolboxFileTest extends AbstractToolboxFileTest {
6766
whenClose
6867
whenClose
6968
thenExpectZippedDirectoryNotExist
70-
thenExpectContentsExists(false)
71-
thenExpectLayoutContentExists(false)
69+
thenExpectContentsNotExists()
7270
])
7371
}
7472

@@ -79,7 +77,7 @@ class ZippedToolboxFileTest extends AbstractToolboxFileTest {
7977
def void testAutoclose() throws Throwable {
8078
MockPlanProVersionService.mockPlanProVersionService([
8179
PlanProPackage.eINSTANCE.eClass();
82-
org.eclipse.set.model.planpro.PlanPro.PlanProPackage.eINSTANCE.
80+
PlanProPackage.eINSTANCE.
8381
eClass();
8482

8583
ToolboxFileRole.SESSION.whenOpenAndAutoclose
@@ -97,23 +95,22 @@ class ZippedToolboxFileTest extends AbstractToolboxFileTest {
9795
def void testCloseThenOpen() throws Throwable{
9896
MockPlanProVersionService.mockPlanProVersionService([
9997
whenOpen
100-
thenExpectContentsExists(true)
101-
thenExpectLayoutContentExists(true)
98+
thenExpectContentsExists()
99+
thenExpectLayoutContentExists()
102100
whenClose
103-
thenExpectContentsExists(false)
104-
thenExpectLayoutContentExists(false)
101+
thenExpectContentsNotExists()
105102
whenOpen
106-
thenExpectContentsExists(true)
107-
thenExpectLayoutContentExists(true)
103+
thenExpectContentsExists()
104+
thenExpectLayoutContentExists()
108105
thenExpectResourceCallsWithinZipDirectory
109106

110107
])
111108
}
112109

113-
def void thenExpectLayoutContentExists(boolean exists) {
110+
def void thenExpectLayoutContentExists() {
114111
if (Files.exists(testee.layoutPath)) {
115112
assertNotNull(testee.layoutResource);
116-
assertTrue(testee.layoutResource.contents.empty != exists)
113+
assertFalse(testee.layoutResource.contents.empty)
117114
}
118115
}
119116

‎java/bundles/org.eclipse.set.core/src/org/eclipse/set/core/fileservice/AbstractToolboxFile.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,11 @@ public void save() throws IOException {
154154

155155
@Override
156156
public void close() throws IOException {
157+
getEditingDomain().getCommandStack().flush();
157158
getEditingDomain().getResourceSet().getResources().clear();
158159
resources.values().forEach(resource -> resource.getContents().clear());
160+
resources.clear();
161+
clearXMLDocument();
159162
}
160163

161164
@Override

‎java/bundles/org.eclipse.set.feature.validation/src/org/eclipse/set/feature/validation/parts/ValidationPart.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import org.eclipse.swt.widgets.Listener;
5656
import org.eclipse.swt.widgets.Shell;
5757

58+
import jakarta.annotation.PostConstruct;
5859
import jakarta.annotation.PreDestroy;
5960
import jakarta.inject.Inject;
6061

@@ -110,6 +111,13 @@ public class ValidationPart extends AbstractEmfFormsPart {
110111
private Listener resizeListener;
111112
private Composite resizeListenerObject;
112113

114+
@PostConstruct
115+
void postConstruct() {
116+
getBroker().subscribe(Events.CLOSE_SESSION, event -> {
117+
modelService.put(INJECT_VIEW_VALIDATION_NATTABLE, null);
118+
});
119+
}
120+
113121
/**
114122
* Create the part.
115123
*/

‎java/bundles/org.eclipse.set.feature.validation/src/org/eclipse/set/feature/validation/session/ModelSession.java‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -298,11 +298,10 @@ public void cancelMergeMode(final Predicate<Path> confirmDeletion)
298298
public void cleanUp() {
299299
try {
300300
// clean sessions subdirectory
301-
cleanSessionDirectory(Paths.get(getSessionsSubDir()));
302-
301+
cleanSessionDirectory(Paths.get(getSessionsSubDir()), getGuid());
303302
// remove content adapter
304303
removeContentAdapter(getPlanProSchnittstelle());
305-
304+
removeContentAdapter(getLayoutInformation());
306305
// unsubscribe event handler
307306
ToolboxEvents.unsubscribe(serviceProvider.broker,
308307
selectedControlAreaChangedEventHandler);
@@ -312,8 +311,8 @@ public void cleanUp() {
312311
}
313312
}
314313

315-
private static void cleanSessionDirectory(final Path unzipDirectory)
316-
throws IOException {
314+
private static void cleanSessionDirectory(final Path unzipDirectory,
315+
final String guid) throws IOException {
317316
if (!Files.exists(unzipDirectory)) {
318317
return;
319318
}
@@ -338,7 +337,9 @@ private static void cleanSessionDirectory(final Path unzipDirectory)
338337
// this acceptable, as over time all directories will be
339338
// cleaned up
340339
final long pid = Long.parseLong(Files.readString(pidPath));
341-
return ProcessHandle.of(pid).isEmpty();
340+
return ProcessHandle.of(pid).isEmpty()
341+
|| ProcessHandle.current().pid() == pid
342+
&& p.getFileName().toString().equals(guid);
342343
} catch (NumberFormatException | IOException e) {
343344
return true;
344345
}
@@ -354,8 +355,6 @@ private static void cleanSessionDirectory(final Path unzipDirectory)
354355

355356
@Override
356357
public void close() {
357-
// flush the command stack
358-
getToolboxFile().getEditingDomain().getCommandStack().flush();
359358
// reset filename
360359
setTitleFilename(null);
361360

‎java/bundles/org.eclipse.set.feature.validation/src/org/eclipse/set/feature/validation/session/SetSessionService.java‎

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
package org.eclipse.set.feature.validation.session;
1111

1212
import java.nio.file.Path;
13+
import java.util.EnumMap;
1314
import java.util.HashMap;
1415
import java.util.List;
1516
import java.util.Map;
@@ -45,6 +46,8 @@
4546
import org.eclipse.set.model.planpro.PlanPro.PlanProPackage;
4647
import org.eclipse.swt.widgets.Shell;
4748
import org.osgi.service.component.annotations.Component;
49+
import org.slf4j.Logger;
50+
import org.slf4j.LoggerFactory;
4851

4952
import com.google.common.collect.Lists;
5053
import com.google.common.collect.Sets;
@@ -57,7 +60,7 @@
5760

5861
@Component(service = SessionService.class)
5962
public class SetSessionService implements SessionService {
60-
63+
Logger logger = LoggerFactory.getLogger(SetSessionService.class);
6164
protected static final Map<String, Set<ToolboxFileExtension>> PLAIN_SUPPORT_MAP;
6265

6366
protected static final Map<String, Set<ToolboxFileExtension>> ZIPPED_SUPPORT_MAP;
@@ -93,7 +96,7 @@ public class SetSessionService implements SessionService {
9396
ZIPPED_SUPPORT_MAP.put(ToolboxConstants.EXTENSION_CATEGORY_PPMERGE,
9497
ppmerge);
9598

96-
loadedModels = new HashMap<>();
99+
loadedModels = new EnumMap<>(ToolboxFileRole.class);
97100
}
98101

99102
protected static String getDefaultExtensionPlainPlanPro() {
@@ -132,34 +135,44 @@ protected static String getDefaultExtensionZippedPlanPro() {
132135
@Override
133136
public boolean close(final IModelSession modelSession,
134137
final ToolboxFileRole role) {
135-
if (role == ToolboxFileRole.COMPARE_PLANNING) {
136-
serviceProvider.broker.send(Events.CLOSE_SESSION, role);
137-
loadedModels.remove(role);
138-
return true;
139-
}
140-
141138
if (modelSession == null) {
142-
serviceProvider.broker.send(Events.CLOSE_SESSION, role);
143-
loadedModels.clear();
144-
return true;
139+
return false;
145140
}
146141

147142
// show the no session part
148143
if (getToolboxPartService()
149-
.showPart(ToolboxConstants.NO_SESSION_PART_ID)) {
150-
// clean up
151-
modelSession.cleanUp();
144+
.showPart(ToolboxConstants.NO_SESSION_PART_ID)
145+
|| role == ToolboxFileRole.SESSION) {
146+
loadedModels.keySet().forEach(this::closeLoadedSession);
147+
152148
actionItems.clear();
153149
// clean up the view area
154150
getToolboxPartService().clean();
155151

156152
// remove the session from the application context
157153
getApplication().getContext().set(IModelSession.class, null);
158154
loadedModels.clear();
159-
serviceProvider.broker.send(Events.CLOSE_SESSION, role);
160155
return true;
161156
}
162157

158+
return closeLoadedSession(role);
159+
}
160+
161+
private boolean closeLoadedSession(final ToolboxFileRole role) {
162+
if (loadedModels.containsKey(role)) {
163+
try {
164+
final IModelSession session = loadedModels.get(role);
165+
if (session != null) {
166+
serviceProvider.broker.send(Events.CLOSE_SESSION, role);
167+
session.getToolboxFile().close();
168+
session.cleanUp();
169+
}
170+
return true;
171+
} catch (final Exception e) {
172+
logger.error(e.getMessage());
173+
return false;
174+
}
175+
}
163176
return false;
164177
}
165178

0 commit comments

Comments
 (0)