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

Container attributes are discarded on re-initialization #625

Open
jsbarber opened this issue Oct 4, 2019 · 3 comments

Comments

@jsbarber
Copy link
Contributor

commented Oct 4, 2019

I'm not completely sure this is a bug, but it was certainly hard to figure out. If it's not a bug, it seems like it's well worth having a big fat warning somewhere on the documentation page.

Essentially it's this. Given the table below:

global my_table: table[count] of string &create_expire=5sec;
...
event some_other_event()
{
    my_table[1] = "foo";
}

event bro_init()
{
    my_table = table();
}

The entry added in some_other_event will never expire. The re-initialization of my_table to an empty table discards all the attributes.

It's just counterintuitive to me that re-initializing a variable in effect changes its type.

@jsbarber

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2019

Demonstration code that can be run on try.zeek.org with ssh.pcap (for example) as background.

redef table_expire_interval = 1sec;

function tt_expire(tt: table[count] of string, key: count): interval
{
    print "EXPIRE", key, tt[key];
    return 0sec;
}

global tt1: table[count] of string &create_expire=2sec &expire_func=tt_expire; 
global tt2: table[count] of string &create_expire=2sec &expire_func=tt_expire;

global serial: count = 0;

event fnext()
{
    serial += 1;
    tt1[serial] = "tt1";
    tt2[serial] = "tt2";
    schedule 1sec { fnext() };
}

event ffirst()
{
    schedule 1sec { fnext() };
}

event bro_init()
{
    tt2 = table();
    schedule 1sec { ffirst() };
}
@rsmmr

This comment has been minimized.

Copy link
Member

commented Oct 7, 2019

Yeah, this is unfortunate. It's a known, broader issue with attributes. Fixing that has bested a few people over time already unfortunately, see #149.

Do you have a suggestion what a good place would be to document the behavior?

@jsbarber

This comment has been minimized.

Copy link
Contributor Author

commented Oct 10, 2019

Even after looking at that other ticket, and reading the https://docs.zeek.org/en/stable/script-reference/attributes.html page again, I don't think I understand the issue well enough to give a great answer here. It appears that attributes are actually being applied to the initial value of the container rather than the variable holding the value?

Anyway, I think a good first step would be a warning on the Attributes page, just below the initial attribute list. Because if a developer finds that the attributes are not working as expected, the first thing they're likely to try is to go back and re-read the documentation looking for behavioral hints. Possibly also a cross-reference in the Types page where the affected types are described.

@jsiwek jsiwek added this to Unassigned / Todo in Release 3.1.0 via automation Oct 14, 2019
@jsiwek jsiwek added this to the 3.1.0 milestone Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Release 3.1.0
  
Unassigned / Todo
3 participants
You can’t perform that action at this time.