Possible bug on Zig's homepage Queue implementation?

Hello,

Right on Zig’s homepage is a generic Queue implementation that I’m leveraging in a project with a minor change to make it thread safe by adding a mutex.

I noticed however, that the dequeue method seems questionable at least in my mind.

pub fn dequeue(self: *Self) ?Child {
            // I added the lock and defer unlock.
            self.mu.lock(); 
            defer self.mu.unlock();

            const start = self.start orelse return null;
            defer self.gpa.destroy(start);
            if (start.next) |next|
                self.start = next
            else {
                self.start = null;
                self.end = null;
            }
            return start.data;
        }

Two observations I had a question on:

  1. The original Queue uses This instead of Self. Correct me if I’m wrong but this just seems like a potentially outdated idiom over using Self.
  2. The bigger question I have: I noticed that start.data is returned, but I’m expecting start to no longer be valid since: defer self.gpa.destroy(start) is invoked higher up?

In my mind, the defer must fire at the end of the scope, but the return statement still references start.data? Is this ok to do? If the defer fires AFTER the return statement I suppose this code is sound.

Defer statements are executed after the last statement in the block, including the return statement.
So by the time the defer is executed, start.data has already been copied to the result location.

7 Likes