[
https://issues.apache.org/jira/browse/ZOOKEEPER-1400?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13658452#comment-13658452
]
Marshall McMullen commented on ZOOKEEPER-1400:
----------------------------------------------
Stephen - Ah, you are absolutely correct. I thought you were referring to the
function log_message not the wrapper macros that call into log_message. Yes, I
was using a GCC extension (didn't actually know that wasn't standard to C99).
I've revised the patch to use normal __VA_ARGS__. I think the current method
should work on windows, but as I have no access to a windows machine or
compiler I can't verify that. I assume the pre-commit job doesn't actually try
to build this on windows and that only happens after a jira is committed...
that's a shame.
At any rate, I've updated the patch. I want to see if the unit test fails
again. It did not fail for me locally on my machine. My test is probably too
particular in the number of messages it expects to be logged. Other code paths
may trigger a log message non deterministically and cause the test to fail. If
that's the case I will loosen things up.
> Allow logging via callback instead of raw FILE pointer
> ------------------------------------------------------
>
> Key: ZOOKEEPER-1400
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1400
> Project: ZooKeeper
> Issue Type: Improvement
> Components: c client
> Affects Versions: 3.5.0
> Environment: Linux
> Reporter: Marshall McMullen
> Assignee: Marshall McMullen
> Fix For: 3.5.0
>
> Attachments: ZOOKEEPER-1400.patch, ZOOKEEPER-1400.patch,
> ZOOKEEPER-1400.patch, ZOOKEEPER-1400.patch
>
>
> The existing logging framework inside the C client uses a raw FILE*. Using a
> FILE* is very limiting and potentially dangerous. A safer alternative is to
> just provide a callback that the C client will call for each message. In our
> environment, we saw some really nasty issues with multiple threads all
> connecting to zookeeper via the C Client related to the use of a raw FILE*.
> Specifically, if the FILE * is closed and that file descriptor is reused by
> the kernel before the C client is notified then the C client will use it's
> static global logStream pointer for subsequent logging messages. That FILE*
> is now a loose cannon! In our environment, we saw zookeeper log messages
> ending up in other sockets and even in our core data path. Clearly this is
> dangerous. In our particular case, we'd omitted a call to
> zoo_set_log_stream(NULL) to notify C client that the FILE* has been closed.
> However, even with that bug fixed, there's still a race condition where log
> messages in flight may be sent before the C client is notified of the FILE
> closure, and the same problem can happen.
> Other issues we've seen involved multiple threads, wherein one would close
> the FILE*, and that's a global change that affects all threads connected
> within that process. That's a pretty nasty limitation as well.
> My proposed change is to allow setting a callback for log messages. A
> callback is used in preference to a raw FILE*. If no callback is set, then it
> will fallback to the existing FILE*. If that's not set, then it falls back to
> stderr as it always has.
> While refactoring this code, I removed the need for the double parens in all
> the LOG macros as that wasn't necessary and didn't fit with my new approach.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
