Skip to content

FOGL-6349: Removed PyObject_Repr and Py_CLEAR(readingsList)#623

Closed
pintomax wants to merge 1 commit intoFOGL-5665from
FOGL-5665_north_plugin_fix
Closed

FOGL-6349: Removed PyObject_Repr and Py_CLEAR(readingsList)#623
pintomax wants to merge 1 commit intoFOGL-5665from
FOGL-5665_north_plugin_fix

Conversation

@pintomax
Copy link
Contributor

FOGL-6349: Removed PyObject_Repr and Py_CLEAR(readingsList)

Removed PyObject_Repr and Py_CLEAR(readingsList)
PyObject* pReturn = PyObject_CallObject(method, arg);
Logger::getLogger()->info("%s:%d, pReturn=%p", __FUNCTION__, __LINE__, pReturn);
Py_CLEAR(arg);
Py_CLEAR(readingsList);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please verify that removing this doesn't cause a memory leak? May be try with a high reading rate or observe sending of few 1000s of readings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see only one evident issue with memory which is NOT related to removal of Py_CLEAR(readingsList);

set "Data block size" to 1200 in North Service

and this error comes:

Apr 13 17:46:13 ubuntu20-lenovo Fledge N5[363509] ERROR: http_north: http_north: Bad request error code: 400, reason: Payload block must be a valid json
Apr 13 17:46:13 ubuntu20-lenovo Fledge N5[363509]: INFO: call_plugin_send_coroutine:72, pReturn=0x7fdf86ccf280
Apr 13 17:46:13 ubuntu20-lenovo Fledge N5[363509]: INFO: numSent=0
Apr 13 17:46:13 ubuntu20-lenovo Fledge N5[363509]: DEBUG: C2Py: plugin_send_fn():L426: filtered readings sent 0

This is originated by the called Fledge with http-south plugin:

Apr 13 17:46:13 ubuntu18-server-virt Fledge SS[25827] ERROR: http_south: fledge.plugins.south.http_south.http_south: 400: Payload block must be a valid json#012Traceback (most recent call last):#12 File "/home/ubuntu/source/fledge-FOGL-6100/fledge/python/fledge/plugins/south/http_south/http_south.py", line 367, in render_post#012 payload_block = await request.json()#12 File "/home/ubuntu/.local/lib/python3.6/site-packages/aiohttp/web_request.py", line 583, in json#012 body = await self.text()#12 File "/home/ubuntu/.local/lib/python3.6/site-packages/aiohttp/web_request.py", line 577, in text#012 bytes_body = await self.read()#12 File "/home/ubuntu/.local/lib/python3.6/site-packages/aiohttp/web_request.py", line 568, in read#012 actual_size=body_size#012aiohttp.web_exceptions.HTTPRequestEntityTooLarge: Request Entity Too Large#012#012During handling of the above exception, another exception occurred:#12#012Traceback (most recent call last):#12 File "/home/ubuntu/source/fledge-FOGL-6100/fledge/python/fledge/plugins/south/http_south/http_south.py", line 369, in render_post#012 raise ValueError('Payload block must be a valid json')#012ValueError: Payload block must be a valid json

When this happens the north process memory starts growing.

PyObject* pReturn = PyObject_CallObject(method, arg);
Logger::getLogger()->info("%s:%d, pReturn=%p", __FUNCTION__, __LINE__, pReturn);
Py_CLEAR(arg);
Py_CLEAR(readingsList);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it is safe to call "Py_CLEAR(readingsList)" twice, the second time would have no effect as per documentation here: https://docs.python.org/3/c-api/refcounting.html#c.Py_CLEAR
But yes, there seems to be an extra call to Py_CLEAR() in call_plugin_send_coroutine() or calling function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AmandeepArora
As per @YashTatkondawar recent tests, we see memory growth while sending data to one fledge to another one with base branch FOGL-5665, this PR has not added new issues, but it solves some crashes, mainly the ones with PyObject_Repr

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. So what do you think has helped fix the issue? It doesn't seem additional Py_CLEAR would have caused an issue as per the link I sent. Is it because of removing the PyObject_Repr() code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw some crashes while using the default string "localhost:6683"
and removal of Py_CLEAR(readingsList) seems fixing it.

What we know from @YashTatkondawar tests is PyObject_Repr() removal solves other crashes.

I can restore line L74 so we can merge the removal of PyObject_Repr()

@pintomax pintomax closed this Apr 15, 2022
@pintomax pintomax deleted the FOGL-5665_north_plugin_fix branch April 15, 2022 10:03
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

Successfully merging this pull request may close these issues.

2 participants