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

New logging API hard to use. #6383

Open
bluhm opened this issue Apr 5, 2024 · 3 comments
Open

New logging API hard to use. #6383

bluhm opened this issue Apr 5, 2024 · 3 comments

Comments

@bluhm
Copy link
Contributor

bluhm commented Apr 5, 2024

The logger was part of client or server config. Memory management was easy, it was just part of the config.
Now logging is a pointer to the logger. The C memory management has to be done by the user.

In 1.0 the logger->clear callback did nothing. In 1.3 it was freeing the context. In 1.4 it is freeing the logger struct.
When setting a custom clear callback, now the old callback has to be called, which frees the old logger struct. Then a new logger is malloced and registered in the config logging pointer. This new logger can get a new clear callback that has to finally free the logger struct.

Exchanging the logging pointer does not work. setDefaultConfig() calls UA_EventLoop_new_POSIX() with the logging pointer. It stores a local copy of the logging pointer in UA_EventLoopPOSIX. This means the logger and config logging pointer must not be exchanged, otherwise the eventloop cannot log anymore.

What is the point of introducing logging pointer? Before memory was just there. Now complicated mangement of malloc and free is necessary. And due to pointer copy in event loop, I end up either with memory leak or segmentation fault.

@jpfr
Copy link
Member

jpfr commented Apr 5, 2024

This is a tricky part.
Many plugins want to get an early pointer to logging.
So that problems during their initialization get reported.

In addition we want a stable location for the logger.
Before, people created the config on the heap and then moved it "inside" the server.
This invalidated the logger location and required manual adjustment.

The 1.4 API is fixed now, as the first release candidates are already out.

What we could do on master:
You always need to create an unitialized server first.
Then you have to configure the config-struct "inside" the server.
Copying the server config from a stack location into the server is strongly adivsed against.

@bluhm
Copy link
Contributor Author

bluhm commented Apr 5, 2024

The config is part of the server. Then memory management is easy. When it exists, configure it there.
With the logger it was the same. It always existed. I have no idea how someone could copy it from the heap.
Now with the pointers it is complicated and plugins are broken.

@LinuxZrc
Copy link

LinuxZrc commented Apr 7, 2024

Yes, I think so, expecting new updates and example of new logger. :D

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