Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

removing DDAbstractDatabaseLogger #34

Closed
ehuynh opened this Issue · 5 comments

3 participants

@ehuynh

I'm having difficultly removing DDAbstractDatabaseLogger with the call [DDLog removeLogger:databaseLogger]. The error EXC_BAD_INSTRUCTION occurs when calling dispatch_release(saveTimer) in the - (void)destroySaveTimer method. In the - (void)willRemoveLogger method, the saveTimer is suspended before calling release. It seems that the issue is because there is a unbalanced number of calls to dispatch_suspend and dispatch_resume on saveTimer. Is dispatch_suspend required as dispatch_source_cancel is called in - (void)destroySaveTimer which prevents any further invocation?

@kerusan

I have the same problem when I set the timer to 0.
Since Im no Experienced C programmer I'm not sure if my fix is the right one to do but I added a dispatch_retain since the removeTimer uses a dispatch_release.

- (void)createSuspendedSaveTimer 
{
    if ((saveTimer == NULL) && (saveInterval > 0.0))
    {
        saveTimer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, loggerQueue);
        dispatch_retain(saveTimer);
        dispatch_source_set_event_handler(saveTimer, ^{ @autoreleasepool {

            [self performSaveAndSuspendSaveTimer];

        }});

        saveTimerSuspended = YES;
    }
 }

also the same crash happens in the deleteTimer so I did the same there.

- (void)createAndStartDeleteTimer
{
    if ((deleteTimer == NULL) && (deleteInterval > 0.0) && (maxAge > 0.0))
    {
        deleteTimer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, loggerQueue);
        dispatch_retain(deleteTimer);
        dispatch_source_set_event_handler(deleteTimer, ^{ @autoreleasepool {

            [self performDelete];

        }});

        [self updateDeleteTimer];

        dispatch_resume(deleteTimer);
    }
}```
@robbiehanson

Is dispatch_suspend required (within willRemoveLogger) as dispatch_source_cancel is called
in - (void)destroySaveTimer which prevents any further invocation?

Technically NO, but this wouldn't fix the bug. Because the timer may already be suspended at the moment willRemoveLogger is called.

The saveTimer automatically gets resumed when there are pending log messages that haven't been flushed (written to database). And it automatically gets suspended after the log messages are flushed. This way, if there aren't any log messages pending, the saveTimer isn't running.

The solution is to check to make sure the saveTimer isn't suspended before calling dispatch_release on the saveTimer.

As for the internals of dispatch_release and GCD timers, this was a good explanation:

The crash you're seeing is actually an assertion. Dispatch objects must be fully resumed before the final release,
this is so that the object can navigate through the internal machinery to properly cancel, etc.

I think the order of operations you're looking for is:

dispatch_source_cancel(source); // ATOMIC - guarantees the event handler will not fire again dispatch_resume
dispatch_resume(source);
dispatch_release(source);

@kerusan

I added a dispatch_retain since the removeTimer uses a dispatch_release

The "retainCount" of the timer is 1 after dispatch_source_create. Adding an additional dispatch_retain will increment the retainCount to 2, and will leak the timer object.

@kerusan
@robbiehanson
Owner

Good question. The saveTimer is suspended when there aren't any pending log messages to save. But the deleteTimer is never suspended because existing log messages are always getting older, and we don't know when they're going to expire.

I suppose we could further optimizations to pause the deleteTimer if the database gets cleared. But under normal circumstances (with a steady flow of log messages), it's not likely an issue.

@kerusan

I think that the deleteTimer gets suspended when setting the deleteInterval to 0.

@ArloL ArloL referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.