Skip to content
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

Memory leak in Receive methods #53

Closed
zemlya25 opened this issue May 25, 2022 · 10 comments
Closed

Memory leak in Receive methods #53

zemlya25 opened this issue May 25, 2022 · 10 comments
Assignees

Comments

@zemlya25
Copy link

zemlya25 commented May 25, 2022

Hi, @matrober-uk

I've realized that current implementation of Receive* methods cause memory leakage. It can't be detected with pprof because of CGO
500mb on 100k received message
Little investigation below, Can prepare PR

func (consumer ConsumerImpl) receiveInternal(gmo *ibmmq.MQGMO)
returns TextMessageImpl/BytesMessageImpl with thisMessageHandle *ibmmq.MQMessageHandle

// Include the message properties in the msgHandle gmo.Options |= ibmmq.MQGMO_PROPERTIES_IN_HANDLE cmho := ibmmq.NewMQCMHO() **thisMsgHandle, _ := consumer.ctx.qMgr.CrtMH(cmho)** gmo.MsgHandle = thisMsgHandle ... msg = &TextMessageImpl{ bodyStr: msgBodyStr, MessageImpl: MessageImpl{ mqmd: getmqmd, **msgHandle: &thisMsgHandle,** }, }

and it should be closed/deleted with msgHandle.DltMH(ibmmq.NewMQDMHO()) after message has been processed

`// Delete message handler
func (msg *MessageImpl) CleanMH() jms20subset.JMSException {
if msg.msgHandle == nil {
return jms20subset.CreateJMSException("EmptyMessageHandler", "EmptyMessageHandler", nil)
}

if err := msg.msgHandle.DltMH(ibmmq.NewMQDMHO()); err != nil {
	return jms20subset.CreateJMSException("UnexpectedDeleteMessageHandlerError", "UnexpectedDeleteMessageHandlerError", err)
}

return nil

}
`

@matrober-uk
Copy link
Member

Hi @zemlya25 - good spot, thank you.
If you'd like to prepare a PR as you suggested I'll be happy to review it for you.

There looks to be an equivalent opportunity for a leak when Sending messages based on calls to createMsgHandle that it would be good to close at the same time, although it's not as clear in that case where it becomes safe to clean up the message handle!
https://github.com/ibm-messaging/mq-golang-jms20/blob/main/mqjms/ContextImpl.go#L184

@magengbin
Copy link

magengbin commented Dec 29, 2022

There are also leaks here, Especially when the message is not available
https://github.com/ibm-messaging/mq-golang-jms20/blob/main/mqjms/ConsumerImpl.go#L87

@matrober-uk matrober-uk self-assigned this Jan 21, 2023
matrober-uk added a commit to matrober-uk/mq-golang-jms20 that referenced this issue Jan 21, 2023
@matrober-uk
Copy link
Member

Basic testing using the Mac activity monitor to check the total process size when running a test;

  • 35,000 receive requests (with no message available)
  • 54.7MB process size before fix
  • 14.0MB process size after fix, so significant improvement

Remaining test to validate during send + receive activity before merging.

matrober-uk added a commit to matrober-uk/mq-golang-jms20 that referenced this issue Jan 22, 2023
matrober-uk added a commit to matrober-uk/mq-golang-jms20 that referenced this issue Jan 22, 2023
@matrober-uk
Copy link
Member

TestLeakOnPutGet

  • 25,000 put/get pairs
    • 46.9MB final process size with just the receive fix
    • 15.1MB final process size with the send fix also

@matrober-uk
Copy link
Member

Merged to main under #63.

Hi @zemlya25 - I've (finally) fixed the memory leaks you reported, if you have some time would you like to pull the latest from the "main" branch and see if it resolves the issue for you?

Best regards, Matt

@zemlya25
Copy link
Author

great news! thanx
still has some memory leak issue on prod system after my local fix with DltMH
I'll check it with new jms version

@zemlya25
Copy link
Author

checked an issue and fix. Works great!

@matrober-uk
Copy link
Member

great, thanks for confirming @zemlya25 - please do feel free to raise any other issues you may find!

@zemlya25
Copy link
Author

@matrober-uk do you have any plans on next release version with implemented fixes?

@matrober-uk
Copy link
Member

Hi @zemlya25 - no time like the present :-)

I've just created a new v1.10.0 release here which includes these memory leak fixes
https://github.com/ibm-messaging/mq-golang-jms20/releases/tag/v1.10.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants