This repository has been archived by the owner on Oct 26, 2021. It is now read-only.
Partial revert of 3b28999fa9a1f7f8cd9b55ba6f8165e2447038fc #7
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
martin-ejdestig
added a commit
to Pelagicore/meta-pelux
that referenced
this pull request
Jul 15, 2019
In applications that link against persistence-common-object. See commit message in patch for details. Upstream pr: GENIVI/persistence-common-object#7 Signed-off-by: Martin Ejdestig <[email protected]>
|
|
@martin-ejdestig Looks like this is a necessary change based on your testing and I'm inclined to merge (in lieu of an active maintainer). Would you consider changing the commit message as proposed (or will it cause issues in your downstream (meta-pelux)). |
Partial revert of 3b28999. Specifically the "Added library constructor/destructor to call DLT_REGISTER_CONTEXT/DLT_UNREGISTER_CONTEXT" part. Originally made in src/key-value-store and copied to src/sqlite so revert change there as well. Calling DLT_REGISTER_CONTEXT in a constructor function is problematic because: - DLT documentation specifically mentions in multiple places that DLT_REGISTER_CONTEXT must not be called before DLT_REGISTER_APP. Unclear what the consequences are if this is done. DLT_REGISTER_APP in main() or a function called from main() will be invoked after constructor functions. - Services that fork() can not use DLT even if they themselves are careful not to perform any calls to DLT until after fork(). This is supported and specifically tested for in DLT. DLT_REGISTER_CONTEXT from constructor function will call dlt_init(), that monitors for fork() with pthread_atfork(), in wrong process. End result is that nothing is ever sent from the main daemon process to the DLT daemon. Have verified that DLT logging in persistence-administrator, that currently daemonizes by calling fork() at startup, does not work without this patch but works with it applied. Signed-off-by: Martin Ejdestig <[email protected]>
|
Updated commit message. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Revert "Added library constructor/destructor to call DLT_REGISTER_CONTEXT/DLT_UNREGISTER_CONTEXT" part. Originally made in src/key-value-store and copied to src/sqlite so revert change there as well.
Calling DLT_REGISTER_CONTEXT in a constructor function is problematic because:
DLT documentation specifically mentions in multiple places that DLT_REGISTER_CONTEXT must not be called before DLT_REGISTER_APP. Unclear what the consequences are if this is done.
Services that fork() can not use DLT even if they themselves are careful not to perform any calls to DLT until after fork(). This is supported and specifically tested for in DLT. DLT_REGISTER_CONTEXT from constructor function will call dlt_init(), that monitors for fork() with pthread_atfork(), in wrong process. End result is that nothing is ever sent from the main daemon process to the DLT daemon.
Have verified that DLT logging in persistence-administrator does not work without this patch but works with it applied.