New issue

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

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

Already on GitHub? Sign in to your account

Recreating Client instance will cause an out-of-memory error #1400

Closed
processing-bugs opened this Issue Feb 10, 2013 · 3 comments

Comments

Projects
None yet
2 participants
@processing-bugs

processing-bugs commented Feb 10, 2013

Original author: hya...@gmail.com (October 31, 2012 08:32:17)

If possible, please attach the smallest possible sketch that reproduces
your problem. First make a tiny example, then use Tools -> Archive Sketch
to create a zip file, and then attach it to this bug report.

What steps will reproduce the problem?

  1. run the sketch ClientServerTest
  2. open process monitor application such as Activity Monitor (MacO X) to check the real memory size continuously increasing
  3. wait for a few minutes; application will stop with out-of-memory error

What is the expected output? What do you see instead?

no real memory size continuous increase, no out-of-memory error.

What version of the product are you using? On what operating system?

processing 1.5.1, 2.0b5 on MacOS X

Please provide any additional information below.

dispose() method defined in Client.java seems not unregistering the "dispose" this object.
Adding the following fragments of code at the beginning of the dispose() would fix this issue.
N.B. 1.5.1 PApplet does not have unregisterMethod(), so the following code is for 2.0+ only.

    // If this Client object was created with the Client(PApplet parent, String host, int port) constructor,
    // unregister this from the parent.
if (host != null)
        parent.unregisterMethod("dispose", this);

Note: Please do not link to locations on other sites, such as your own
image site, video site, blog, etc. Include the relevant information here
with the report.

Original issue: http://code.google.com/p/processing/issues/detail?id=1362

@processing-bugs

This comment has been minimized.

Show comment
Hide comment
@processing-bugs

processing-bugs Feb 10, 2013

From f...@processing.org on November 24, 2012 18:47:42
From issue 189, there's a note that the fix suggested here has a bad interaction with the fix applied there.

processing-bugs commented Feb 10, 2013

From f...@processing.org on November 24, 2012 18:47:42
From issue 189, there's a note that the fix suggested here has a bad interaction with the fix applied there.

@processing-bugs

This comment has been minimized.

Show comment
Hide comment
@processing-bugs

processing-bugs Feb 10, 2013

From hyamagu...@acm.org on December 12, 2012 07:51:03
In Client(PApplet parent, String host, int port) constructor, if its associated socket,
in, out, and thread instances are successfully created, then
parent.registerMethod("dispose", this);
will be executed and this object reference will be stored in the parent's HashMap.

This object will never been gc'ed unless unregistered, which would cause an OutOfMemory
error.

The correct location of calling the unregisterMethod() is not in the dispose() method,
but in the stop(). We need to check whether this object is actually registered before
unregistering it. if the host is non-null, then the object was created the above
constructor, and if thread is non-null, parent.registerMethod("dispose", this) is executed.

Please check the code fragment below:

public void stop() {

  • // if parent.registerMethod("dispose", this) was done in construction
  • // undo it here
  • if (host != null && thread != null)
  • parent.unregisterMethod("dispose", this);
  • if (disconnectEventMethod != null) {
    try {
    disconnectEventMethod.invoke(parent, new Object[] { this });
    } catch (Exception e) {
    e.printStackTrace();
    disconnectEventMethod = null;
    }
    }
    dispose();
    }

This will go along with the latest 2.0b7 code, which incorporates issue 189 patch.

processing-bugs commented Feb 10, 2013

From hyamagu...@acm.org on December 12, 2012 07:51:03
In Client(PApplet parent, String host, int port) constructor, if its associated socket,
in, out, and thread instances are successfully created, then
parent.registerMethod("dispose", this);
will be executed and this object reference will be stored in the parent's HashMap.

This object will never been gc'ed unless unregistered, which would cause an OutOfMemory
error.

The correct location of calling the unregisterMethod() is not in the dispose() method,
but in the stop(). We need to check whether this object is actually registered before
unregistering it. if the host is non-null, then the object was created the above
constructor, and if thread is non-null, parent.registerMethod("dispose", this) is executed.

Please check the code fragment below:

public void stop() {

  • // if parent.registerMethod("dispose", this) was done in construction
  • // undo it here
  • if (host != null && thread != null)
  • parent.unregisterMethod("dispose", this);
  • if (disconnectEventMethod != null) {
    try {
    disconnectEventMethod.invoke(parent, new Object[] { this });
    } catch (Exception e) {
    e.printStackTrace();
    disconnectEventMethod = null;
    }
    }
    dispose();
    }

This will go along with the latest 2.0b7 code, which incorporates issue 189 patch.

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry Apr 1, 2015

Member

Fixed by #3088

Member

benfry commented Apr 1, 2015

Fixed by #3088

@benfry benfry closed this Apr 1, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment